From e375107aa4f7407c8cdbbeefb0d82435e5f4cfad Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 12 Sep 2018 11:23:38 +0200 Subject: [PATCH] Redirect guests to login if they follow the link of a comment mention-notification Signed-off-by: Joas Schilling --- .../comments/lib/Controller/Notifications.php | 46 +++--- .../Unit/Controller/NotificationsTest.php | 132 ++++++++++++------ 2 files changed, 121 insertions(+), 57 deletions(-) diff --git a/apps/comments/lib/Controller/Notifications.php b/apps/comments/lib/Controller/Notifications.php index 911f19d46e..3eeba4c866 100644 --- a/apps/comments/lib/Controller/Notifications.php +++ b/apps/comments/lib/Controller/Notifications.php @@ -31,8 +31,10 @@ use OCP\AppFramework\Http\Response; use OCP\Comments\IComment; use OCP\Comments\ICommentsManager; use OCP\Files\Folder; +use OCP\Files\IRootFolder; use OCP\IRequest; use OCP\IURLGenerator; +use OCP\IUser; use OCP\IUserSession; use OCP\Notification\IManager; @@ -42,8 +44,8 @@ use OCP\Notification\IManager; * @package OCA\Comments\Controller */ class Notifications extends Controller { - /** @var Folder */ - protected $folder; + /** @var IRootFolder */ + protected $rootFolder; /** @var ICommentsManager */ protected $commentsManager; @@ -63,7 +65,7 @@ class Notifications extends Controller { * @param string $appName * @param IRequest $request * @param ICommentsManager $commentsManager - * @param Folder $folder + * @param IRootFolder $rootFolder * @param IURLGenerator $urlGenerator * @param IManager $notificationManager * @param IUserSession $userSession @@ -72,35 +74,50 @@ class Notifications extends Controller { $appName, IRequest $request, ICommentsManager $commentsManager, - Folder $folder, + IRootFolder $rootFolder, IURLGenerator $urlGenerator, IManager $notificationManager, IUserSession $userSession ) { parent::__construct($appName, $request); $this->commentsManager = $commentsManager; - $this->folder = $folder; + $this->rootFolder = $rootFolder; $this->urlGenerator = $urlGenerator; $this->notificationManager = $notificationManager; $this->userSession = $userSession; } /** - * @NoAdminRequired + * @PublicPage * @NoCSRFRequired * * @param string $id the comment ID * @return Response */ public function view($id) { + $currentUser = $this->userSession->getUser(); + if (!$currentUser instanceof IUser) { + return new RedirectResponse( + $this->urlGenerator->linkToRoute('core.login.showLoginForm', [ + 'redirect_url' => $this->urlGenerator->linkToRoute( + 'comments.Notifications.view', + ['id' => $id] + ), + ]) + ); + } + try { $comment = $this->commentsManager->get($id); if($comment->getObjectType() !== 'files') { return new NotFoundResponse(); } - $files = $this->folder->getById((int)$comment->getObjectId()); - if(count($files) === 0) { - $this->markProcessed($comment); + $userFolder = $this->rootFolder->getUserFolder($currentUser->getUID()); + $files = $userFolder->getById((int)$comment->getObjectId()); + + $this->markProcessed($comment, $currentUser); + + if (empty($files)) { return new NotFoundResponse(); } @@ -109,8 +126,6 @@ class Notifications extends Controller { [ 'fileid' => $comment->getObjectId() ] ); - $this->markProcessed($comment); - return new RedirectResponse($url); } catch (\Exception $e) { return new NotFoundResponse(); @@ -120,17 +135,14 @@ class Notifications extends Controller { /** * Marks the notification about a comment as processed * @param IComment $comment + * @param IUser $currentUser */ - protected function markProcessed(IComment $comment) { - $user = $this->userSession->getUser(); - if(is_null($user)) { - return; - } + protected function markProcessed(IComment $comment, IUser $currentUser) { $notification = $this->notificationManager->createNotification(); $notification->setApp('comments') ->setObject('comment', $comment->getId()) ->setSubject('mention') - ->setUser($user->getUID()); + ->setUser($currentUser->getUID()); $this->notificationManager->markProcessed($notification); } } diff --git a/apps/comments/tests/Unit/Controller/NotificationsTest.php b/apps/comments/tests/Unit/Controller/NotificationsTest.php index eb29947342..9897027b82 100644 --- a/apps/comments/tests/Unit/Controller/NotificationsTest.php +++ b/apps/comments/tests/Unit/Controller/NotificationsTest.php @@ -24,10 +24,13 @@ namespace OCA\Comments\Tests\Unit\Controller; use OCA\Comments\Controller\Notifications; +use OCP\AppFramework\Http\NotFoundResponse; +use OCP\AppFramework\Http\RedirectResponse; use OCP\Comments\IComment; use OCP\Comments\ICommentsManager; use OCP\Comments\NotFoundException; use OCP\Files\Folder; +use OCP\Files\IRootFolder; use OCP\Files\Node; use OCP\IRequest; use OCP\IURLGenerator; @@ -38,75 +41,117 @@ use OCP\Notification\INotification; use Test\TestCase; class NotificationsTest extends TestCase { - /** @var \OCA\Comments\Controller\Notifications */ + /** @var Notifications */ protected $notificationsController; - /** @var \OCP\Comments\ICommentsManager|\PHPUnit_Framework_MockObject_MockObject */ + /** @var ICommentsManager|\PHPUnit_Framework_MockObject_MockObject */ protected $commentsManager; - /** @var \OCP\Files\Folder|\PHPUnit_Framework_MockObject_MockObject */ - protected $folder; + /** @var IRootFolder|\PHPUnit_Framework_MockObject_MockObject */ + protected $rootFolder; - /** @var \OCP\IUserSession|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IUserSession|\PHPUnit_Framework_MockObject_MockObject */ protected $session; - /** @var \OCP\Notification\IManager|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IManager|\PHPUnit_Framework_MockObject_MockObject */ protected $notificationManager; + /** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */ + protected $urlGenerator; + protected function setUp() { parent::setUp(); - $this->commentsManager = $this->getMockBuilder(ICommentsManager::class)->getMock(); - $this->folder = $this->getMockBuilder(Folder::class)->getMock(); - $this->session = $this->getMockBuilder(IUserSession::class)->getMock(); - $this->notificationManager = $this->getMockBuilder(IManager::class)->getMock(); + $this->commentsManager = $this->createMock(ICommentsManager::class); + $this->rootFolder = $this->createMock(IRootFolder::class); + $this->session = $this->createMock(IUserSession::class); + $this->notificationManager = $this->createMock(IManager::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->notificationsController = new Notifications( 'comments', - $this->getMockBuilder(IRequest::class)->getMock(), + $this->createMock(IRequest::class), $this->commentsManager, - $this->folder, - $this->getMockBuilder(IURLGenerator::class)->getMock(), + $this->rootFolder, + $this->urlGenerator, $this->notificationManager, $this->session ); } - + + public function testViewGuestRedirect() { + $this->commentsManager->expects($this->never()) + ->method('get'); + + $this->rootFolder->expects($this->never()) + ->method('getUserFolder'); + + $this->session->expects($this->once()) + ->method('getUser') + ->willReturn(null); + + $this->notificationManager->expects($this->never()) + ->method('createNotification'); + $this->notificationManager->expects($this->never()) + ->method('markProcessed'); + + $this->urlGenerator->expects($this->exactly(2)) + ->method('linkToRoute') + ->withConsecutive( + ['comments.Notifications.view', ['id' => '42']], + ['core.login.showLoginForm', ['redirect_url' => 'link-to-comment']] + ) + ->willReturnMap([ + ['comments.Notifications.view', ['id' => '42'], 'link-to-comment'], + ['core.login.showLoginForm', ['redirect_url' => 'link-to-comment'], 'link-to-login'], + ]); + + /** @var RedirectResponse $response */ + $response = $this->notificationsController->view('42'); + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame('link-to-login', $response->getRedirectURL()); + } + public function testViewSuccess() { - $comment = $this->getMockBuilder(IComment::class)->getMock(); + $comment = $this->createMock(IComment::class); $comment->expects($this->any()) ->method('getObjectType') - ->will($this->returnValue('files')); + ->willReturn('files'); $this->commentsManager->expects($this->any()) ->method('get') ->with('42') - ->will($this->returnValue($comment)); + ->willReturn($comment); - $file = $this->getMockBuilder(Node::class)->getMock(); + $file = $this->createMock(Node::class); + $folder = $this->createMock(Folder::class); - $this->folder->expects($this->once()) + $this->rootFolder->expects($this->once()) + ->method('getUserFolder') + ->willReturn($folder); + + $folder->expects($this->once()) ->method('getById') - ->will($this->returnValue([$file])); + ->willReturn([$file]); $this->session->expects($this->once()) ->method('getUser') - ->will($this->returnValue($this->getMockBuilder(IUser::class)->getMock())); + ->willReturn($this->createMock(IUser::class)); - $notification = $this->getMockBuilder(INotification::class)->getMock(); + $notification = $this->createMock(INotification::class); $notification->expects($this->any()) ->method($this->anything()) - ->will($this->returnValue($notification)); + ->willReturn($notification); $this->notificationManager->expects($this->once()) ->method('createNotification') - ->will($this->returnValue($notification)); + ->willReturn($notification); $this->notificationManager->expects($this->once()) ->method('markProcessed') ->with($notification); $response = $this->notificationsController->view('42'); - $this->assertInstanceOf('\OCP\AppFramework\Http\RedirectResponse', $response); + $this->assertInstanceOf(RedirectResponse::class, $response); } public function testViewInvalidComment() { @@ -115,11 +160,12 @@ class NotificationsTest extends TestCase { ->with('42') ->will($this->throwException(new NotFoundException())); - $this->folder->expects($this->never()) - ->method('getById'); + $this->rootFolder->expects($this->never()) + ->method('getUserFolder'); - $this->session->expects($this->never()) - ->method('getUser'); + $this->session->expects($this->once()) + ->method('getUser') + ->willReturn($this->createMock(IUser::class)); $this->notificationManager->expects($this->never()) ->method('createNotification'); @@ -127,41 +173,47 @@ class NotificationsTest extends TestCase { ->method('markProcessed'); $response = $this->notificationsController->view('42'); - $this->assertInstanceOf('\OCP\AppFramework\Http\NotFoundResponse', $response); + $this->assertInstanceOf(NotFoundResponse::class, $response); } public function testViewNoFile() { - $comment = $this->getMockBuilder(IComment::class)->getMock(); + $comment = $this->createMock(IComment::class); $comment->expects($this->any()) ->method('getObjectType') - ->will($this->returnValue('files')); + ->willReturn('files'); $this->commentsManager->expects($this->any()) ->method('get') ->with('42') - ->will($this->returnValue($comment)); + ->willReturn($comment); - $this->folder->expects($this->once()) + $folder = $this->createMock(Folder::class); + + $this->rootFolder->expects($this->once()) + ->method('getUserFolder') + ->willReturn($folder); + + $folder->expects($this->once()) ->method('getById') - ->will($this->returnValue([])); + ->willReturn([]); $this->session->expects($this->once()) ->method('getUser') - ->will($this->returnValue($this->getMockBuilder(IUser::class)->getMock())); + ->willReturn($this->createMock(IUser::class)); - $notification = $this->getMockBuilder(INotification::class)->getMock(); + $notification = $this->createMock(INotification::class); $notification->expects($this->any()) ->method($this->anything()) - ->will($this->returnValue($notification)); + ->willReturn($notification); $this->notificationManager->expects($this->once()) ->method('createNotification') - ->will($this->returnValue($notification)); + ->willReturn($notification); $this->notificationManager->expects($this->once()) ->method('markProcessed') ->with($notification); $response = $this->notificationsController->view('42'); - $this->assertInstanceOf('\OCP\AppFramework\Http\NotFoundResponse', $response); + $this->assertInstanceOf(NotFoundResponse::class, $response); } }