From 3bd95cca6ba614c6af77af4c33c09cc6b152729c Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Fri, 19 Feb 2016 19:45:03 +0100 Subject: [PATCH] Check if user has permission to create such a tag Fixes https://github.com/owncloud/core/issues/22512 --- apps/dav/lib/server.php | 6 +- apps/dav/lib/systemtag/systemtagnode.php | 1 + apps/dav/lib/systemtag/systemtagplugin.php | 29 +- .../tests/unit/systemtag/systemtagplugin.php | 255 +++++++++++++++++- 4 files changed, 285 insertions(+), 6 deletions(-) diff --git a/apps/dav/lib/server.php b/apps/dav/lib/server.php index fd18d0d21ac..74be318fe5e 100644 --- a/apps/dav/lib/server.php +++ b/apps/dav/lib/server.php @@ -93,7 +93,11 @@ class Server { $this->server->addPlugin(new \OCA\DAV\CardDAV\Plugin()); // system tags plugins - $this->server->addPlugin(new \OCA\DAV\SystemTag\SystemTagPlugin(\OC::$server->getSystemTagManager())); + $this->server->addPlugin(new \OCA\DAV\SystemTag\SystemTagPlugin( + \OC::$server->getSystemTagManager(), + \OC::$server->getGroupManager(), + \OC::$server->getUserSession() + )); // comments plugin $this->server->addPlugin(new \OCA\DAV\Comments\CommentsPlugin( diff --git a/apps/dav/lib/systemtag/systemtagnode.php b/apps/dav/lib/systemtag/systemtagnode.php index ecdb39a762c..7a47a752ad0 100644 --- a/apps/dav/lib/systemtag/systemtagnode.php +++ b/apps/dav/lib/systemtag/systemtagnode.php @@ -103,6 +103,7 @@ class SystemTagNode implements \Sabre\DAV\INode { * @param bool $userVisible user visible * @param bool $userAssignable user assignable * @throws NotFound whenever the given tag id does not exist + * @throws Forbidden whenever there is no permission to update said tag * @throws Conflict whenever a tag already exists with the given attributes */ public function update($name, $userVisible, $userAssignable) { diff --git a/apps/dav/lib/systemtag/systemtagplugin.php b/apps/dav/lib/systemtag/systemtagplugin.php index 3348b431c47..7da24ba7cf8 100644 --- a/apps/dav/lib/systemtag/systemtagplugin.php +++ b/apps/dav/lib/systemtag/systemtagplugin.php @@ -21,6 +21,8 @@ */ namespace OCA\DAV\SystemTag; +use OCP\IGroupManager; +use OCP\IUserSession; use Sabre\DAV\Exception\NotFound; use Sabre\DAV\PropFind; use Sabre\DAV\PropPatch; @@ -61,12 +63,26 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin { protected $tagManager; /** - * System tags plugin - * + * @var IUserSession + */ + protected $userSession; + + /** + * @var IGroupManager + */ + protected $groupManager; + + /** * @param ISystemTagManager $tagManager tag manager + * @param IGroupManager $groupManager + * @param IUserSession $userSession */ - public function __construct(ISystemTagManager $tagManager) { + public function __construct(ISystemTagManager $tagManager, + IGroupManager $groupManager, + IUserSession $userSession) { $this->tagManager = $tagManager; + $this->userSession = $userSession; + $this->groupManager = $groupManager; } /** @@ -163,6 +179,13 @@ class SystemTagPlugin extends \Sabre\DAV\ServerPlugin { if (isset($data['userAssignable'])) { $userAssignable = (bool)$data['userAssignable']; } + + if($userVisible === false || $userAssignable === false) { + if(!$this->userSession->isLoggedIn() || !$this->groupManager->isAdmin($this->userSession->getUser()->getUID())) { + throw new BadRequest('Not sufficient permissions'); + } + } + try { return $this->tagManager->createTag($tagName, $userVisible, $userAssignable); } catch (TagAlreadyExistsException $e) { diff --git a/apps/dav/tests/unit/systemtag/systemtagplugin.php b/apps/dav/tests/unit/systemtag/systemtagplugin.php index b026451701f..b945223e668 100644 --- a/apps/dav/tests/unit/systemtag/systemtagplugin.php +++ b/apps/dav/tests/unit/systemtag/systemtagplugin.php @@ -22,6 +22,8 @@ namespace OCA\DAV\Tests\Unit\SystemTag; use OC\SystemTag\SystemTag; +use OCP\IGroupManager; +use OCP\IUserSession; use OCP\SystemTag\TagAlreadyExistsException; class SystemTagPlugin extends \Test\TestCase { @@ -46,6 +48,16 @@ class SystemTagPlugin extends \Test\TestCase { */ private $tagManager; + /** + * @var IGroupManager + */ + private $groupManager; + + /** + * @var IUserSession + */ + private $userSession; + /** * @var \OCA\DAV\SystemTag\SystemTagPlugin */ @@ -60,8 +72,14 @@ class SystemTagPlugin extends \Test\TestCase { $this->server = new \Sabre\DAV\Server($this->tree); $this->tagManager = $this->getMock('\OCP\SystemTag\ISystemTagManager'); + $this->groupManager = $this->getMock('\OCP\IGroupManager'); + $this->userSession = $this->getMock('\OCP\IUserSession'); - $this->plugin = new \OCA\DAV\SystemTag\SystemTagPlugin($this->tagManager); + $this->plugin = new \OCA\DAV\SystemTag\SystemTagPlugin( + $this->tagManager, + $this->groupManager, + $this->userSession + ); $this->plugin->initialize($this->server); } @@ -153,7 +171,204 @@ class SystemTagPlugin extends \Test\TestCase { $this->assertEquals(200, $result[self::USERVISIBLE_PROPERTYNAME]); } + /** + * @expectedException \Sabre\DAV\Exception\BadRequest + * @expectedExceptionMessage Not sufficient permissions + */ + public function testCreateNotAssignableTagAsRegularUser() { + $user = $this->getMock('\OCP\IUser'); + $user->expects($this->once()) + ->method('getUID') + ->willReturn('admin'); + $this->userSession + ->expects($this->once()) + ->method('isLoggedIn') + ->willReturn(true); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->willReturn($user); + $this->groupManager + ->expects($this->once()) + ->method('isAdmin') + ->with('admin') + ->willReturn(false); + + $requestData = json_encode([ + 'name' => 'Test', + 'userVisible' => true, + 'userAssignable' => false, + ]); + + $node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagsByIdCollection') + ->disableOriginalConstructor() + ->getMock(); + $this->tagManager->expects($this->never()) + ->method('createTag'); + + $this->tree->expects($this->any()) + ->method('getNodeForPath') + ->with('/systemtags') + ->will($this->returnValue($node)); + + $request = $this->getMockBuilder('Sabre\HTTP\RequestInterface') + ->disableOriginalConstructor() + ->getMock(); + $response = $this->getMockBuilder('Sabre\HTTP\ResponseInterface') + ->disableOriginalConstructor() + ->getMock(); + + $request->expects($this->once()) + ->method('getPath') + ->will($this->returnValue('/systemtags')); + + $request->expects($this->once()) + ->method('getBodyAsString') + ->will($this->returnValue($requestData)); + + $request->expects($this->once()) + ->method('getHeader') + ->with('Content-Type') + ->will($this->returnValue('application/json')); + + $this->plugin->httpPost($request, $response); + } + + /** + * @expectedException \Sabre\DAV\Exception\BadRequest + * @expectedExceptionMessage Not sufficient permissions + */ + public function testCreateInvisibleTagAsRegularUser() { + $user = $this->getMock('\OCP\IUser'); + $user->expects($this->once()) + ->method('getUID') + ->willReturn('admin'); + $this->userSession + ->expects($this->once()) + ->method('isLoggedIn') + ->willReturn(true); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->willReturn($user); + $this->groupManager + ->expects($this->once()) + ->method('isAdmin') + ->with('admin') + ->willReturn(false); + + $requestData = json_encode([ + 'name' => 'Test', + 'userVisible' => false, + 'userAssignable' => true, + ]); + + $node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagsByIdCollection') + ->disableOriginalConstructor() + ->getMock(); + $this->tagManager->expects($this->never()) + ->method('createTag'); + + $this->tree->expects($this->any()) + ->method('getNodeForPath') + ->with('/systemtags') + ->will($this->returnValue($node)); + + $request = $this->getMockBuilder('Sabre\HTTP\RequestInterface') + ->disableOriginalConstructor() + ->getMock(); + $response = $this->getMockBuilder('Sabre\HTTP\ResponseInterface') + ->disableOriginalConstructor() + ->getMock(); + + $request->expects($this->once()) + ->method('getPath') + ->will($this->returnValue('/systemtags')); + + $request->expects($this->once()) + ->method('getBodyAsString') + ->will($this->returnValue($requestData)); + + $request->expects($this->once()) + ->method('getHeader') + ->with('Content-Type') + ->will($this->returnValue('application/json')); + + $this->plugin->httpPost($request, $response); + } + + public function testCreateTagInByIdCollectionAsRegularUser() { + $systemTag = new SystemTag(1, 'Test', true, false); + + $requestData = json_encode([ + 'name' => 'Test', + 'userVisible' => true, + 'userAssignable' => true, + ]); + + $node = $this->getMockBuilder('\OCA\DAV\SystemTag\SystemTagsByIdCollection') + ->disableOriginalConstructor() + ->getMock(); + $this->tagManager->expects($this->once()) + ->method('createTag') + ->with('Test', true, true) + ->will($this->returnValue($systemTag)); + + $this->tree->expects($this->any()) + ->method('getNodeForPath') + ->with('/systemtags') + ->will($this->returnValue($node)); + + $request = $this->getMockBuilder('Sabre\HTTP\RequestInterface') + ->disableOriginalConstructor() + ->getMock(); + $response = $this->getMockBuilder('Sabre\HTTP\ResponseInterface') + ->disableOriginalConstructor() + ->getMock(); + + $request->expects($this->once()) + ->method('getPath') + ->will($this->returnValue('/systemtags')); + + $request->expects($this->once()) + ->method('getBodyAsString') + ->will($this->returnValue($requestData)); + + $request->expects($this->once()) + ->method('getHeader') + ->with('Content-Type') + ->will($this->returnValue('application/json')); + + $request->expects($this->once()) + ->method('getUrl') + ->will($this->returnValue('http://example.com/dav/systemtags')); + + $response->expects($this->once()) + ->method('setHeader') + ->with('Content-Location', 'http://example.com/dav/systemtags/1'); + + $this->plugin->httpPost($request, $response); + } + public function testCreateTagInByIdCollection() { + $user = $this->getMock('\OCP\IUser'); + $user->expects($this->once()) + ->method('getUID') + ->willReturn('admin'); + $this->userSession + ->expects($this->once()) + ->method('isLoggedIn') + ->willReturn(true); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->willReturn($user); + $this->groupManager + ->expects($this->once()) + ->method('isAdmin') + ->with('admin') + ->willReturn(true); + $systemTag = new SystemTag(1, 'Test', true, false); $requestData = json_encode([ @@ -214,6 +429,24 @@ class SystemTagPlugin extends \Test\TestCase { } public function testCreateTagInMappingCollection() { + $user = $this->getMock('\OCP\IUser'); + $user->expects($this->once()) + ->method('getUID') + ->willReturn('admin'); + $this->userSession + ->expects($this->once()) + ->method('isLoggedIn') + ->willReturn(true); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->willReturn($user); + $this->groupManager + ->expects($this->once()) + ->method('isAdmin') + ->with('admin') + ->willReturn(true); + $systemTag = new SystemTag(1, 'Test', true, false); $requestData = json_encode([ @@ -307,9 +540,27 @@ class SystemTagPlugin extends \Test\TestCase { /** * @dataProvider nodeClassProvider - * @expectedException Sabre\DAV\Exception\Conflict + * @expectedException \Sabre\DAV\Exception\Conflict */ public function testCreateTagConflict($nodeClass) { + $user = $this->getMock('\OCP\IUser'); + $user->expects($this->once()) + ->method('getUID') + ->willReturn('admin'); + $this->userSession + ->expects($this->once()) + ->method('isLoggedIn') + ->willReturn(true); + $this->userSession + ->expects($this->once()) + ->method('getUser') + ->willReturn($user); + $this->groupManager + ->expects($this->once()) + ->method('isAdmin') + ->with('admin') + ->willReturn(true); + $requestData = json_encode([ 'name' => 'Test', 'userVisible' => true, -- 2.39.5