diff --git a/apps/dav/lib/comments/commentnode.php b/apps/dav/lib/comments/commentnode.php index a5d508dd19..d3cd53bceb 100644 --- a/apps/dav/lib/comments/commentnode.php +++ b/apps/dav/lib/comments/commentnode.php @@ -27,6 +27,7 @@ use OCP\Comments\ICommentsManager; use OCP\ILogger; use OCP\IUserManager; use OCP\IUserSession; +use Sabre\DAV\Exception\Forbidden; use Sabre\DAV\Exception\MethodNotAllowed; use Sabre\DAV\PropPatch; @@ -112,12 +113,23 @@ class CommentNode implements \Sabre\DAV\INode, \Sabre\DAV\IProperties { ]; } + protected function checkWriteAccessOnComment() { + $user = $this->userSession->getUser(); + if( $this->comment->getActorType() !== 'users' + || is_null($user) + || $this->comment->getActorId() !== $user->getUID() + ) { + throw new Forbidden('Only authors are allowed to edit their comment.'); + } + } + /** * Deleted the current node * * @return void */ function delete() { + $this->checkWriteAccessOnComment(); $this->commentsManager->delete($this->comment->getId()); } @@ -156,8 +168,10 @@ class CommentNode implements \Sabre\DAV\INode, \Sabre\DAV\IProperties { * * @param $propertyValue * @return bool + * @throws Forbidden */ public function updateComment($propertyValue) { + $this->checkWriteAccessOnComment(); try { $this->comment->setMessage($propertyValue); $this->commentsManager->save($this->comment); diff --git a/apps/dav/tests/unit/comments/commentnode.php b/apps/dav/tests/unit/comments/commentnode.php index 44ac54ae93..8d1bf06ab6 100644 --- a/apps/dav/tests/unit/comments/commentnode.php +++ b/apps/dav/tests/unit/comments/commentnode.php @@ -51,10 +51,28 @@ class CommentsNode extends \Test\TestCase { } public function testDelete() { + $user = $this->getMock('\OCP\IUser'); + + $user->expects($this->once()) + ->method('getUID') + ->will($this->returnValue('alice')); + + $this->userSession->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($user)); + $this->comment->expects($this->once()) ->method('getId') ->will($this->returnValue('19')); + $this->comment->expects($this->any()) + ->method('getActorType') + ->will($this->returnValue('users')); + + $this->comment->expects($this->any()) + ->method('getActorId') + ->will($this->returnValue('alice')); + $this->commentsManager->expects($this->once()) ->method('delete') ->with('19'); @@ -62,6 +80,37 @@ class CommentsNode extends \Test\TestCase { $this->node->delete(); } + /** + * @expectedException \Sabre\DAV\Exception\Forbidden + */ + public function testDeleteForbidden() { + $user = $this->getMock('\OCP\IUser'); + + $user->expects($this->once()) + ->method('getUID') + ->will($this->returnValue('mallory')); + + $this->userSession->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($user)); + + $this->comment->expects($this->never()) + ->method('getId'); + + $this->comment->expects($this->any()) + ->method('getActorType') + ->will($this->returnValue('users')); + + $this->comment->expects($this->any()) + ->method('getActorId') + ->will($this->returnValue('alice')); + + $this->commentsManager->expects($this->never()) + ->method('delete'); + + $this->node->delete(); + } + public function testGetName() { $id = '19'; $this->comment->expects($this->once()) @@ -85,10 +134,28 @@ class CommentsNode extends \Test\TestCase { public function testUpdateComment() { $msg = 'Hello Earth'; + $user = $this->getMock('\OCP\IUser'); + + $user->expects($this->once()) + ->method('getUID') + ->will($this->returnValue('alice')); + + $this->userSession->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($user)); + $this->comment->expects($this->once()) ->method('setMessage') ->with($msg); + $this->comment->expects($this->any()) + ->method('getActorType') + ->will($this->returnValue('users')); + + $this->comment->expects($this->any()) + ->method('getActorId') + ->will($this->returnValue('alice')); + $this->commentsManager->expects($this->once()) ->method('save') ->with($this->comment); @@ -96,14 +163,32 @@ class CommentsNode extends \Test\TestCase { $this->assertTrue($this->node->updateComment($msg)); } - public function testUpdateCommentException() { + public function testUpdateCommentLogException() { $msg = null; + $user = $this->getMock('\OCP\IUser'); + + $user->expects($this->once()) + ->method('getUID') + ->will($this->returnValue('alice')); + + $this->userSession->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($user)); + $this->comment->expects($this->once()) ->method('setMessage') ->with($msg) ->will($this->throwException(new \Exception('buh!'))); + $this->comment->expects($this->any()) + ->method('getActorType') + ->will($this->returnValue('users')); + + $this->comment->expects($this->any()) + ->method('getActorId') + ->will($this->returnValue('alice')); + $this->commentsManager->expects($this->never()) ->method('save'); @@ -113,6 +198,90 @@ class CommentsNode extends \Test\TestCase { $this->assertFalse($this->node->updateComment($msg)); } + /** + * @expectedException \Sabre\DAV\Exception\Forbidden + */ + public function testUpdateForbiddenByUser() { + $msg = 'HaXX0r'; + + $user = $this->getMock('\OCP\IUser'); + + $user->expects($this->once()) + ->method('getUID') + ->will($this->returnValue('mallory')); + + $this->userSession->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($user)); + + $this->comment->expects($this->never()) + ->method('setMessage'); + + $this->comment->expects($this->any()) + ->method('getActorType') + ->will($this->returnValue('users')); + + $this->comment->expects($this->any()) + ->method('getActorId') + ->will($this->returnValue('alice')); + + $this->commentsManager->expects($this->never()) + ->method('save'); + + $this->node->updateComment($msg); + } + + /** + * @expectedException \Sabre\DAV\Exception\Forbidden + */ + public function testUpdateForbiddenByType() { + $msg = 'HaXX0r'; + + $user = $this->getMock('\OCP\IUser'); + + $user->expects($this->never()) + ->method('getUID'); + + $this->userSession->expects($this->once()) + ->method('getUser') + ->will($this->returnValue($user)); + + $this->comment->expects($this->never()) + ->method('setMessage'); + + $this->comment->expects($this->any()) + ->method('getActorType') + ->will($this->returnValue('bots')); + + $this->commentsManager->expects($this->never()) + ->method('save'); + + $this->node->updateComment($msg); + } + + /** + * @expectedException \Sabre\DAV\Exception\Forbidden + */ + public function testUpdateForbiddenByNotLoggedIn() { + $msg = 'HaXX0r'; + + $this->userSession->expects($this->once()) + ->method('getUser') + ->will($this->returnValue(null)); + + $this->comment->expects($this->never()) + ->method('setMessage'); + + $this->comment->expects($this->any()) + ->method('getActorType') + ->will($this->returnValue('users')); + + $this->commentsManager->expects($this->never()) + ->method('save'); + + $this->node->updateComment($msg); + } + public function testPropPatch() { $propPatch = $this->getMockBuilder('Sabre\DAV\PropPatch') ->disableOriginalConstructor()