From 5963128342cc6ae386a1a59d81784d0fb4a88fd4 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 13 May 2016 11:07:21 +0200 Subject: Adjust DAV SystemTagPlugin unit tests for groups --- apps/dav/lib/SystemTag/SystemTagPlugin.php | 41 ++-- apps/dav/tests/unit/systemtag/systemtagplugin.php | 282 +++++++++++++++++----- 2 files changed, 244 insertions(+), 79 deletions(-) (limited to 'apps/dav') diff --git a/apps/dav/lib/SystemTag/SystemTagPlugin.php b/apps/dav/lib/SystemTag/SystemTagPlugin.php index fa6010a5bf5..af683954b87 100644 --- a/apps/dav/lib/SystemTag/SystemTagPlugin.php +++ b/apps/dav/lib/SystemTag/SystemTagPlugin.php @@ -24,12 +24,13 @@ namespace OCA\DAV\SystemTag; use OCP\IGroupManager; use OCP\IUserSession; -use Sabre\DAV\Exception\NotFound; use Sabre\DAV\PropFind; use Sabre\DAV\PropPatch; use Sabre\DAV\Exception\BadRequest; -use Sabre\DAV\Exception\UnsupportedMediaType; use Sabre\DAV\Exception\Conflict; +use Sabre\DAV\Exception\Forbidden; +use Sabre\DAV\Exception\NotFound; +use Sabre\DAV\Exception\UnsupportedMediaType; use OCP\SystemTag\ISystemTag; use OCP\SystemTag\ISystemTagManager; @@ -246,16 +247,18 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin { return $this->tagManager->canUserAssignTag($node->getSystemTag(), $this->userSession->getUser()) ? 'true' : 'false'; }); - if ($this->groupManager->isAdmin($this->userSession->getUser()->getUID())) { - $propFind->handle(self::GROUPS_PROPERTYNAME, function() use ($node) { - $groups = []; - // no need to retrieve groups for namespaces that don't qualify - if ($node->getSystemTag()->isUserVisible() && !$node->getSystemTag()->isUserAssignable()) { - $groups = $this->tagManager->getTagGroups($node->getSystemTag()); - } - return implode('|', $groups); - }); - } + $propFind->handle(self::GROUPS_PROPERTYNAME, function() use ($node) { + if (!$this->groupManager->isAdmin($this->userSession->getUser()->getUID())) { + // property only available for admins + throw new Forbidden(); + } + $groups = []; + // no need to retrieve groups for namespaces that don't qualify + if ($node->getSystemTag()->isUserVisible() && !$node->getSystemTag()->isUserAssignable()) { + $groups = $this->tagManager->getTagGroups($node->getSystemTag()); + } + return implode('|', $groups); + }); } /** @@ -302,15 +305,21 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin { $updateTag = true; } - if ($updateTag) { - $node->update($name, $userVisible, $userAssignable); - } - if (isset($props[self::GROUPS_PROPERTYNAME])) { + if (!$this->groupManager->isAdmin($this->userSession->getUser()->getUID())) { + // property only available for admins + throw new Forbidden(); + } + $propValue = $props[self::GROUPS_PROPERTYNAME]; $groupIds = explode('|', $propValue); $this->tagManager->setTagGroups($tag, $groupIds); } + + if ($updateTag) { + $node->update($name, $userVisible, $userAssignable); + } + return true; }); diff --git a/apps/dav/tests/unit/systemtag/systemtagplugin.php b/apps/dav/tests/unit/systemtag/systemtagplugin.php index edf7b6fb43d..da82bc8904a 100644 --- a/apps/dav/tests/unit/systemtag/systemtagplugin.php +++ b/apps/dav/tests/unit/systemtag/systemtagplugin.php @@ -28,6 +28,7 @@ use OCP\IGroupManager; use OCP\IUserSession; use OCP\SystemTag\TagAlreadyExistsException; use OCP\IUser; +use OCP\SystemTag\ISystemTag; class SystemTagPlugin extends \Test\TestCase { @@ -35,6 +36,8 @@ class SystemTagPlugin extends \Test\TestCase { const DISPLAYNAME_PROPERTYNAME = \OCA\DAV\SystemTag\SystemTagPlugin::DISPLAYNAME_PROPERTYNAME; const USERVISIBLE_PROPERTYNAME = \OCA\DAV\SystemTag\SystemTagPlugin::USERVISIBLE_PROPERTYNAME; const USERASSIGNABLE_PROPERTYNAME = \OCA\DAV\SystemTag\SystemTagPlugin::USERASSIGNABLE_PROPERTYNAME; + const CANASSIGN_PROPERTYNAME = \OCA\DAV\SystemTag\SystemTagPlugin::CANASSIGN_PROPERTYNAME; + const GROUPS_PROPERTYNAME = \OCA\DAV\SystemTag\SystemTagPlugin::GROUPS_PROPERTYNAME; /** * @var \Sabre\DAV\Server @@ -100,22 +103,84 @@ class SystemTagPlugin extends \Test\TestCase { $this->plugin->initialize($this->server); } - public function testGetProperties() { - $systemTag = new SystemTag(1, 'Test', true, true); - $requestedProperties = [ - self::ID_PROPERTYNAME, - self::DISPLAYNAME_PROPERTYNAME, - self::USERVISIBLE_PROPERTYNAME, - self::USERASSIGNABLE_PROPERTYNAME - ]; - $expectedProperties = [ - 200 => [ - self::ID_PROPERTYNAME => '1', - self::DISPLAYNAME_PROPERTYNAME => 'Test', - self::USERVISIBLE_PROPERTYNAME => 'true', - self::USERASSIGNABLE_PROPERTYNAME => 'true', - ] + public function getPropertiesDataProvider() { + return [ + [ + new SystemTag(1, 'Test', true, true), + [], + [ + self::ID_PROPERTYNAME, + self::DISPLAYNAME_PROPERTYNAME, + self::USERVISIBLE_PROPERTYNAME, + self::USERASSIGNABLE_PROPERTYNAME, + self::CANASSIGN_PROPERTYNAME, + ], + [ + self::ID_PROPERTYNAME => '1', + self::DISPLAYNAME_PROPERTYNAME => 'Test', + self::USERVISIBLE_PROPERTYNAME => 'true', + self::USERASSIGNABLE_PROPERTYNAME => 'true', + self::CANASSIGN_PROPERTYNAME => 'true', + ] + ], + [ + new SystemTag(1, 'Test', true, false), + [], + [ + self::ID_PROPERTYNAME, + self::DISPLAYNAME_PROPERTYNAME, + self::USERVISIBLE_PROPERTYNAME, + self::USERASSIGNABLE_PROPERTYNAME, + self::CANASSIGN_PROPERTYNAME, + ], + [ + self::ID_PROPERTYNAME => '1', + self::DISPLAYNAME_PROPERTYNAME => 'Test', + self::USERVISIBLE_PROPERTYNAME => 'true', + self::USERASSIGNABLE_PROPERTYNAME => 'false', + self::CANASSIGN_PROPERTYNAME => 'false', + ] + ], + [ + new SystemTag(1, 'Test', true, false), + ['group1', 'group2'], + [ + self::ID_PROPERTYNAME, + self::GROUPS_PROPERTYNAME, + ], + [ + self::ID_PROPERTYNAME => '1', + self::GROUPS_PROPERTYNAME => 'group1|group2', + ] + ], + [ + new SystemTag(1, 'Test', true, true), + ['group1', 'group2'], + [ + self::ID_PROPERTYNAME, + self::GROUPS_PROPERTYNAME, + ], + [ + self::ID_PROPERTYNAME => '1', + // groups only returned when userAssignable is false + self::GROUPS_PROPERTYNAME => '', + ] + ], ]; + } + + /** + * @dataProvider getPropertiesDataProvider + */ + public function testGetProperties(ISystemTag $systemTag, $groups, $requestedProperties, $expectedProperties) { + $this->user->expects($this->any()) + ->method('getUID') + ->willReturn('admin'); + $this->groupManager + ->expects($this->any()) + ->method('isAdmin') + ->with('admin') + ->willReturn(true); $node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagNode') ->disableOriginalConstructor() @@ -124,6 +189,14 @@ class SystemTagPlugin extends \Test\TestCase { ->method('getSystemTag') ->will($this->returnValue($systemTag)); + $this->tagManager->expects($this->any()) + ->method('canUserAssignTag') + ->will($this->returnValue($systemTag->isUserAssignable())); + + $this->tagManager->expects($this->any()) + ->method('getTagGroups') + ->will($this->returnValue($groups)); + $this->tree->expects($this->any()) ->method('getNodeForPath') ->with('/systemtag/1') @@ -143,12 +216,62 @@ class SystemTagPlugin extends \Test\TestCase { $result = $propFind->getResultForMultiStatus(); $this->assertEmpty($result[404]); - unset($result[404]); - $this->assertEquals($expectedProperties, $result); + $this->assertEquals($expectedProperties, $result[200]); + } + + /** + * @expectedException \Sabre\DAV\Exception\Forbidden + */ + public function testGetPropertiesForbidden() { + $systemTag = new SystemTag(1, 'Test', true, false); + $requestedProperties = [ + self::ID_PROPERTYNAME, + self::GROUPS_PROPERTYNAME, + ]; + $this->user->expects($this->once()) + ->method('getUID') + ->willReturn('admin'); + $this->groupManager + ->expects($this->once()) + ->method('isAdmin') + ->with('admin') + ->willReturn(false); + + $node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagNode') + ->disableOriginalConstructor() + ->getMock(); + $node->expects($this->any()) + ->method('getSystemTag') + ->will($this->returnValue($systemTag)); + + $this->tree->expects($this->any()) + ->method('getNodeForPath') + ->with('/systemtag/1') + ->will($this->returnValue($node)); + + $propFind = new \Sabre\DAV\PropFind( + '/systemtag/1', + $requestedProperties, + 0 + ); + + $this->plugin->handleGetProperties( + $propFind, + $node + ); } - public function testUpdateProperties() { + public function testUpdatePropertiesAdmin() { $systemTag = new SystemTag(1, 'Test', true, false); + $this->user->expects($this->any()) + ->method('getUID') + ->willReturn('admin'); + $this->groupManager + ->expects($this->any()) + ->method('isAdmin') + ->with('admin') + ->willReturn(true); + $node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagNode') ->disableOriginalConstructor() ->getMock(); @@ -165,11 +288,16 @@ class SystemTagPlugin extends \Test\TestCase { ->method('update') ->with('Test changed', false, true); + $this->tagManager->expects($this->once()) + ->method('setTagGroups') + ->with($systemTag, ['group1', 'group2']); + // properties to set $propPatch = new \Sabre\DAV\PropPatch(array( self::DISPLAYNAME_PROPERTYNAME => 'Test changed', self::USERVISIBLE_PROPERTYNAME => 'false', self::USERASSIGNABLE_PROPERTYNAME => 'true', + self::GROUPS_PROPERTYNAME => 'group1|group2', )); $this->plugin->handleUpdateProperties( @@ -189,64 +317,63 @@ class SystemTagPlugin extends \Test\TestCase { } /** - * @expectedException \Sabre\DAV\Exception\BadRequest - * @expectedExceptionMessage Not sufficient permissions + * @expectedException \Sabre\DAV\Exception\Forbidden */ - public function testCreateNotAssignableTagAsRegularUser() { - $this->user->expects($this->once()) + public function testUpdatePropertiesForbidden() { + $systemTag = new SystemTag(1, 'Test', true, false); + $this->user->expects($this->any()) ->method('getUID') ->willReturn('admin'); $this->groupManager - ->expects($this->once()) + ->expects($this->any()) ->method('isAdmin') ->with('admin') ->willReturn(false); - $requestData = json_encode([ - 'name' => 'Test', - 'userVisible' => true, - 'userAssignable' => false, - ]); - - $node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagsByIdCollection') + $node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagNode') ->disableOriginalConstructor() ->getMock(); - $this->tagManager->expects($this->never()) - ->method('createTag'); + $node->expects($this->any()) + ->method('getSystemTag') + ->will($this->returnValue($systemTag)); $this->tree->expects($this->any()) ->method('getNodeForPath') - ->with('/systemtags') + ->with('/systemtag/1') ->will($this->returnValue($node)); - $request = $this->getMockBuilder('Sabre\HTTP\RequestInterface') - ->disableOriginalConstructor() - ->getMock(); - $response = $this->getMockBuilder('Sabre\HTTP\ResponseInterface') - ->disableOriginalConstructor() - ->getMock(); + $node->expects($this->never()) + ->method('update'); - $request->expects($this->once()) - ->method('getPath') - ->will($this->returnValue('/systemtags')); + $this->tagManager->expects($this->never()) + ->method('setTagGroups'); - $request->expects($this->once()) - ->method('getBodyAsString') - ->will($this->returnValue($requestData)); + // properties to set + $propPatch = new \Sabre\DAV\PropPatch(array( + self::GROUPS_PROPERTYNAME => 'group1|group2', + )); - $request->expects($this->once()) - ->method('getHeader') - ->with('Content-Type') - ->will($this->returnValue('application/json')); + $this->plugin->handleUpdateProperties( + '/systemtag/1', + $propPatch + ); - $this->plugin->httpPost($request, $response); + $propPatch->commit(); } + public function createTagInsufficientPermissionsProvider() { + return [ + [true, false, ''], + [false, true, ''], + [true, true, 'group1|group2'], + ]; + } /** + * @dataProvider createTagInsufficientPermissionsProvider * @expectedException \Sabre\DAV\Exception\BadRequest * @expectedExceptionMessage Not sufficient permissions */ - public function testCreateInvisibleTagAsRegularUser() { + public function testCreateNotAssignableTagAsRegularUser($userVisible, $userAssignable, $groups) { $this->user->expects($this->once()) ->method('getUID') ->willReturn('admin'); @@ -256,17 +383,23 @@ class SystemTagPlugin extends \Test\TestCase { ->with('admin') ->willReturn(false); - $requestData = json_encode([ + $requestData = [ 'name' => 'Test', - 'userVisible' => false, - 'userAssignable' => true, - ]); + 'userVisible' => $userVisible, + 'userAssignable' => $userAssignable, + ]; + if (!empty($groups)) { + $requestData['groups'] = $groups; + } + $requestData = json_encode($requestData); $node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagsByIdCollection') ->disableOriginalConstructor() ->getMock(); $this->tagManager->expects($this->never()) ->method('createTag'); + $this->tagManager->expects($this->never()) + ->method('setTagGroups'); $this->tree->expects($this->any()) ->method('getNodeForPath') @@ -349,7 +482,18 @@ class SystemTagPlugin extends \Test\TestCase { $this->plugin->httpPost($request, $response); } - public function testCreateTagInByIdCollection() { + public function createTagProvider() { + return [ + [true, false, ''], + [false, false, ''], + [true, false, 'group1|group2'], + ]; + } + + /** + * @dataProvider createTagProvider + */ + public function testCreateTagInByIdCollection($userVisible, $userAssignable, $groups) { $this->user->expects($this->once()) ->method('getUID') ->willReturn('admin'); @@ -361,19 +505,33 @@ class SystemTagPlugin extends \Test\TestCase { $systemTag = new SystemTag(1, 'Test', true, false); - $requestData = json_encode([ + $requestData = [ 'name' => 'Test', - 'userVisible' => true, - 'userAssignable' => false, - ]); + 'userVisible' => $userVisible, + 'userAssignable' => $userAssignable, + ]; + if (!empty($groups)) { + $requestData['groups'] = $groups; + } + $requestData = json_encode($requestData); $node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagsByIdCollection') ->disableOriginalConstructor() ->getMock(); $this->tagManager->expects($this->once()) ->method('createTag') - ->with('Test', true, false) + ->with('Test', $userVisible, $userAssignable) ->will($this->returnValue($systemTag)); + + if (!empty($groups)) { + $this->tagManager->expects($this->once()) + ->method('setTagGroups') + ->with($systemTag, explode('|', $groups)) + ->will($this->returnValue($systemTag)); + } else { + $this->tagManager->expects($this->never()) + ->method('setTagGroups'); + } $this->tree->expects($this->any()) ->method('getNodeForPath') @@ -489,8 +647,6 @@ class SystemTagPlugin extends \Test\TestCase { * @expectedException \Sabre\DAV\Exception\NotFound */ public function testCreateTagToUnknownNode() { - $systemTag = new SystemTag(1, 'Test', true, false); - $node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagsObjectMappingCollection') ->disableOriginalConstructor() ->getMock(); -- cgit v1.2.3