Merge pull request #9518 from nextcloud/feature/5986/public_share_controller_middleware

Public share middleware & controller

3 files changed, 26 insertions, 307 deletions

diff --git a/apps/files_sharing/tests/Controller/PublicPreviewControllerTest.php b/apps/files_sharing/tests/Controller/PublicPreviewControllerTest.php
index 174abbb6f60..27e13bc8ced 100644
--- a/apps/files_sharing/tests/Controller/PublicPreviewControllerTest.php
+++ b/apps/files_sharing/tests/Controller/PublicPreviewControllerTest.php
@@ -33,6 +33,7 @@ use OCP\Files\NotFoundException;
 use OCP\Files\SimpleFS\ISimpleFile;
 use OCP\IPreview;
 use OCP\IRequest;
+use OCP\ISession;
 use OCP\Share\Exceptions\ShareNotFound;
 use OCP\Share\IManager;
 use OCP\Share\IShare;
@@ -60,26 +61,27 @@ class PublicPreviewControllerTest extends TestCase {
 			'files_sharing',
 			$this->createMock(IRequest::class),
 			$this->shareManager,
+			$this->createMock(ISession::class),
 			$this->previewManager
 		);
 	}
 
 	public function testInvalidToken() {
-		$res = $this->controller->getPreview('file', 10, 10, '');
+		$res = $this->controller->getPreview('', 'file', 10, 10, '');
 		$expected = new DataResponse([], Http::STATUS_BAD_REQUEST);
 
 		$this->assertEquals($expected, $res);
 	}
 
 	public function testInvalidWidth() {
-		$res = $this->controller->getPreview('file', 0);
+		$res = $this->controller->getPreview('token', 'file', 0);
 		$expected = new DataResponse([], Http::STATUS_BAD_REQUEST);
 
 		$this->assertEquals($expected, $res);
 	}
 
 	public function testInvalidHeight() {
-		$res = $this->controller->getPreview('file', 10, 0);
+		$res = $this->controller->getPreview('token', 'file', 10, 0);
 		$expected = new DataResponse([], Http::STATUS_BAD_REQUEST);
 
 		$this->assertEquals($expected, $res);
@@ -90,7 +92,7 @@ class PublicPreviewControllerTest extends TestCase {
 			->with($this->equalTo('token'))
 			->willThrowException(new ShareNotFound());
 
-		$res = $this->controller->getPreview('file', 10, 10, 'token');
+		$res = $this->controller->getPreview('token', 'file', 10, 10);
 		$expected = new DataResponse([], Http::STATUS_NOT_FOUND);
 
 		$this->assertEquals($expected, $res);
@@ -105,7 +107,7 @@ class PublicPreviewControllerTest extends TestCase {
 		$share->method('getPermissions')
 			->willReturn(0);
 
-		$res = $this->controller->getPreview('file', 10, 10, 'token');
+		$res = $this->controller->getPreview('token', 'file', 10, 10);
 		$expected = new DataResponse([], Http::STATUS_FORBIDDEN);
 
 		$this->assertEquals($expected, $res);
@@ -132,7 +134,7 @@ class PublicPreviewControllerTest extends TestCase {
 		$preview->method('getMimeType')
 			->willReturn('myMime');
 
-		$res = $this->controller->getPreview('file', 10, 10, 'token', true);
+		$res = $this->controller->getPreview('token', 'file', 10, 10, true);
 		$expected = new FileDisplayResponse($preview, Http::STATUS_OK, ['Content-Type' => 'myMime']);
 		$this->assertEquals($expected, $res);
 	}
@@ -154,7 +156,7 @@ class PublicPreviewControllerTest extends TestCase {
 			->with($this->equalTo('file'))
 			->willThrowException(new NotFoundException());
 
-		$res = $this->controller->getPreview('file', 10, 10, 'token', true);
+		$res = $this->controller->getPreview('token', 'file', 10, 10, true);
 		$expected = new DataResponse([], Http::STATUS_NOT_FOUND);
 		$this->assertEquals($expected, $res);
 	}
@@ -186,7 +188,7 @@ class PublicPreviewControllerTest extends TestCase {
 		$preview->method('getMimeType')
 			->willReturn('myMime');
 
-		$res = $this->controller->getPreview('file', 10, 10, 'token', true);
+		$res = $this->controller->getPreview('token', 'file', 10, 10, true);
 		$expected = new FileDisplayResponse($preview, Http::STATUS_OK, ['Content-Type' => 'myMime']);
 		$this->assertEquals($expected, $res);
 	}
diff --git a/apps/files_sharing/tests/Controller/ShareControllerTest.php b/apps/files_sharing/tests/Controller/ShareControllerTest.php
index be99c5ee194..fb417878647 100644
--- a/apps/files_sharing/tests/Controller/ShareControllerTest.php
+++ b/apps/files_sharing/tests/Controller/ShareControllerTest.php
@@ -39,6 +39,7 @@ use OCP\AppFramework\Http\Template\ExternalShareMenuAction;
 use OCP\AppFramework\Http\Template\LinkMenuAction;
 use OCP\AppFramework\Http\Template\PublicTemplateResponse;
 use OCP\AppFramework\Http\Template\SimpleMenuAction;
+use OCP\Files\NotFoundException;
 use OCP\IConfig;
 use OCP\IL10N;
 use OCP\ILogger;
@@ -156,193 +157,24 @@ class ShareControllerTest extends \Test\TestCase {
 		parent::tearDown();
 	}
 
-	public function testShowAuthenticateNotAuthenticated() {
-		$share = \OC::$server->getShareManager()->newShare();
-
-		$this->shareManager
-			->expects($this->once())
-			->method('getShareByToken')
-			->with('token')
-			->willReturn($share);
-
-		$response = $this->shareController->showAuthenticate('token');
-		$expectedResponse =  new TemplateResponse($this->appName, 'authenticate', [], 'guest');
-		$this->assertEquals($expectedResponse, $response);
-	}
-
-	public function testShowAuthenticateAuthenticatedForDifferentShare() {
-		$share = \OC::$server->getShareManager()->newShare();
-		$share->setId(1);
-
-		$this->shareManager
-			->expects($this->once())
-			->method('getShareByToken')
-			->with('token')
-			->willReturn($share);
-
-		$this->session->method('exists')->with('public_link_authenticated')->willReturn(true);
-		$this->session->method('get')->with('public_link_authenticated')->willReturn('2');
-
-		$response = $this->shareController->showAuthenticate('token');
-		$expectedResponse =  new TemplateResponse($this->appName, 'authenticate', [], 'guest');
-		$this->assertEquals($expectedResponse, $response);
-	}
-
-	public function testShowAuthenticateCorrectShare() {
-		$share = \OC::$server->getShareManager()->newShare();
-		$share->setId(1);
-
-		$this->shareManager
-			->expects($this->once())
-			->method('getShareByToken')
-			->with('token')
-			->willReturn($share);
-
-		$this->session->method('exists')->with('public_link_authenticated')->willReturn(true);
-		$this->session->method('get')->with('public_link_authenticated')->willReturn('1');
-
-		$this->urlGenerator->expects($this->once())
-			->method('linkToRoute')
-			->with('files_sharing.sharecontroller.showShare', ['token' => 'token'])
-			->willReturn('redirect');
-
-		$response = $this->shareController->showAuthenticate('token');
-		$expectedResponse =  new RedirectResponse('redirect');
-		$this->assertEquals($expectedResponse, $response);
-	}
-
-	public function testAuthenticateInvalidToken() {
-		$this->shareManager
-			->expects($this->once())
-			->method('getShareByToken')
-			->with('token')
-			->will($this->throwException(new \OCP\Share\Exceptions\ShareNotFound()));
-
-		$response = $this->shareController->authenticate('token', 'preview');
-		$expectedResponse =  new NotFoundResponse();
-		$this->assertEquals($expectedResponse, $response);
-	}
-
-	public function testAuthenticateValidPassword() {
-		$share = \OC::$server->getShareManager()->newShare();
-		$share->setId(42);
-
-		$this->shareManager
-			->expects($this->once())
-			->method('getShareByToken')
-			->with('token')
-			->willReturn($share);
-
-		$this->shareManager
-			->expects($this->once())
-			->method('checkPassword')
-			->with($share, 'validpassword')
-			->willReturn(true);
-
-		$this->session
-			->expects($this->once())
-			->method('set')
-			->with('public_link_authenticated', '42');
-
-		$this->urlGenerator->expects($this->once())
-			->method('linkToRoute')
-			->with('files_sharing.sharecontroller.showShare', ['token'=>'token'])
-			->willReturn('redirect');
-
-		$response = $this->shareController->authenticate('token', 'preview', 'validpassword');
-		$expectedResponse =  new RedirectResponse('redirect');
-		$this->assertEquals($expectedResponse, $response);
-	}
-
-	public function testAuthenticateValidPasswordAndDownload() {
-		$share = \OC::$server->getShareManager()->newShare();
-		$share->setId(42);
-
-		$this->shareManager
-			->expects($this->once())
-			->method('getShareByToken')
-			->with('token')
-			->willReturn($share);
-
-		$this->shareManager
-			->expects($this->once())
-			->method('checkPassword')
-			->with($share, 'validpassword')
-			->willReturn(true);
-
-		$this->session
-			->expects($this->once())
-			->method('set')
-			->with('public_link_authenticated', '42');
-
-		$this->urlGenerator->expects($this->once())
-			->method('linkToRoute')
-			->with('files_sharing.sharecontroller.downloadShare', ['token'=>'token'])
-			->willReturn('redirect');
-
-		$response = $this->shareController->authenticate('token', 'download', 'validpassword');
-		$expectedResponse =  new RedirectResponse('redirect');
-		$this->assertEquals($expectedResponse, $response);
-	}
-
-	public function testAuthenticateInvalidPassword() {
-		$share = \OC::$server->getShareManager()->newShare();
-		$share->setNodeId(100)
-			->setNodeType('file')
-			->setToken('token')
-			->setSharedBy('initiator')
-			->setId(42);
-
-		$this->shareManager
-			->expects($this->once())
-			->method('getShareByToken')
-			->with('token')
-			->willReturn($share);
-
-		$this->shareManager
-			->expects($this->once())
-			->method('checkPassword')
-			->with($share, 'invalidpassword')
-			->willReturn(false);
-
-		$this->session
-			->expects($this->never())
-			->method('set');
-
-		$hookListner = $this->getMockBuilder('Dummy')->setMethods(['access'])->getMock();
-		\OCP\Util::connectHook('OCP\Share', 'share_link_access',  $hookListner, 'access');
-
-		$hookListner->expects($this->once())
-			->method('access')
-			->with($this->callback(function(array $data) {
-				return $data['itemType'] === 'file' &&
-					$data['itemSource'] === 100 &&
-					$data['uidOwner'] === 'initiator' &&
-					$data['token'] === 'token' &&
-					$data['errorCode'] === 403 &&
-					$data['errorMessage'] === 'Wrong password';
-			}));
-
-		$response = $this->shareController->authenticate('token', 'preview', 'invalidpassword');
-		$expectedResponse =  new TemplateResponse($this->appName, 'authenticate', array('wrongpw' => true), 'guest');
-		$expectedResponse->throttle();
-		$this->assertEquals($expectedResponse, $response);
-	}
-
 	public function testShowShareInvalidToken() {
+		$this->shareController->setToken('invalidtoken');
+
 		$this->shareManager
 			->expects($this->once())
 			->method('getShareByToken')
 			->with('invalidtoken')
 			->will($this->throwException(new ShareNotFound()));
 
+		$this->expectException(NotFoundException::class);
+
 		// Test without a not existing token
-		$response = $this->shareController->showShare('invalidtoken');
-		$expectedResponse =  new NotFoundResponse();
-		$this->assertEquals($expectedResponse, $response);
+		$this->shareController->showShare();
 	}
 
 	public function testShowShareNotAuthenticated() {
+		$this->shareController->setToken('validtoken');
+
 		$share = \OC::$server->getShareManager()->newShare();
 		$share->setPassword('password');
 
@@ -352,19 +184,16 @@ class ShareControllerTest extends \Test\TestCase {
 			->with('validtoken')
 			->willReturn($share);
 
-		$this->urlGenerator->expects($this->once())
-			->method('linkToRoute')
-			->with('files_sharing.sharecontroller.authenticate', ['token' => 'validtoken', 'redirect' => 'preview'])
-			->willReturn('redirect');
+		$this->expectException(NotFoundException::class);
 
 		// Test without a not existing token
-		$response = $this->shareController->showShare('validtoken');
-		$expectedResponse = new RedirectResponse('redirect');
-		$this->assertEquals($expectedResponse, $response);
+		$this->shareController->showShare();
 	}
 
 
 	public function testShowShare() {
+		$this->shareController->setToken('token');
+
 		$owner = $this->getMockBuilder(IUser::class)->getMock();
 		$owner->method('getDisplayName')->willReturn('ownerDisplay');
 		$owner->method('getUID')->willReturn('ownerUID');
@@ -428,7 +257,7 @@ class ShareControllerTest extends \Test\TestCase {
 				return vsprintf($text, $parameters);
 			}));
 
-		$response = $this->shareController->showShare('token');
+		$response = $this->shareController->showShare();
 		$sharedTmplParams = array(
 			'displayName' => 'ownerDisplay',
 			'owner' => 'ownerUID',
@@ -476,6 +305,8 @@ class ShareControllerTest extends \Test\TestCase {
 	 * @expectedException \OCP\Files\NotFoundException
 	 */
 	public function testShowShareInvalid() {
+		$this->shareController->setToken('token');
+
 		$owner = $this->getMockBuilder(IUser::class)->getMock();
 		$owner->method('getDisplayName')->willReturn('ownerDisplay');
 		$owner->method('getUID')->willReturn('ownerUID');
@@ -517,32 +348,7 @@ class ShareControllerTest extends \Test\TestCase {
 
 		$this->userManager->method('get')->with('ownerUID')->willReturn($owner);
 
-		$this->shareController->showShare('token');
-	}
-
-	public function testDownloadShare() {
-		$share = $this->getMockBuilder(IShare::class)->getMock();
-		$share->method('getPassword')->willReturn('password');
-		$share
-			->expects($this->once())
-			->method('getPermissions')
-			->willReturn(\OCP\Constants::PERMISSION_READ);
-
-		$this->shareManager
-			->expects($this->once())
-			->method('getShareByToken')
-			->with('validtoken')
-			->willReturn($share);
-
-		$this->urlGenerator->expects($this->once())
-			->method('linkToRoute')
-			->with('files_sharing.sharecontroller.authenticate', ['token' => 'validtoken', 'redirect' => 'download'])
-			->willReturn('redirect');
-
-		// Test with a password protected share and no authentication
-		$response = $this->shareController->downloadShare('validtoken');
-		$expectedResponse = new RedirectResponse('redirect');
-		$this->assertEquals($expectedResponse, $response);
+		$this->shareController->showShare();
 	}
 
 	public function testDownloadShareWithCreateOnlyShare() {
diff --git a/apps/files_sharing/tests/Middleware/SharingCheckMiddlewareTest.php b/apps/files_sharing/tests/Middleware/SharingCheckMiddlewareTest.php
index d8676547a76..1fea73e6b47 100644
--- a/apps/files_sharing/tests/Middleware/SharingCheckMiddlewareTest.php
+++ b/apps/files_sharing/tests/Middleware/SharingCheckMiddlewareTest.php
@@ -98,49 +98,6 @@ class SharingCheckMiddlewareTest extends \Test\TestCase {
 		$this->assertFalse(self::invokePrivate($this->sharingCheckMiddleware, 'isSharingEnabled'));
 	}
 
-	public function testIsLinkSharingEnabledWithEverythinEnabled() {
-		$this->config
-			->expects($this->at(0))
-			->method('getAppValue')
-			->with('core', 'shareapi_enabled', 'yes')
-			->will($this->returnValue('yes'));
-
-		$this->config
-			->expects($this->at(1))
-			->method('getAppValue')
-			->with('core', 'shareapi_allow_links', 'yes')
-			->will($this->returnValue('yes'));
-
-		$this->assertTrue(self::invokePrivate($this->sharingCheckMiddleware, 'isLinkSharingEnabled'));
-	}
-
-
-	public function testIsLinkSharingEnabledWithLinkSharingDisabled() {
-		$this->config
-			->expects($this->at(0))
-			->method('getAppValue')
-			->with('core', 'shareapi_enabled', 'yes')
-			->will($this->returnValue('yes'));
-
-		$this->config
-			->expects($this->at(1))
-			->method('getAppValue')
-			->with('core', 'shareapi_allow_links', 'yes')
-			->will($this->returnValue('no'));
-
-		$this->assertFalse(self::invokePrivate($this->sharingCheckMiddleware, 'isLinkSharingEnabled'));
-	}
-
-	public function testIsLinkSharingEnabledWithSharingAPIDisabled() {
-		$this->config
-			->expects($this->once())
-			->method('getAppValue')
-			->with('core', 'shareapi_enabled', 'yes')
-			->will($this->returnValue('no'));
-
-		$this->assertFalse(self::invokePrivate($this->sharingCheckMiddleware, 'isLinkSharingEnabled'));
-	}
-
 	public function externalSharesChecksDataProvider() {
 
 		$data = [];
@@ -236,25 +193,6 @@ class SharingCheckMiddlewareTest extends \Test\TestCase {
 			->with('files_sharing')
 			->will($this->returnValue(true));
 
-		$this->config
-			->expects($this->at(0))
-			->method('getAppValue')
-			->with('core', 'shareapi_enabled', 'yes')
-			->will($this->returnValue('yes'));
-
-		$this->config
-			->expects($this->at(1))
-			->method('getAppValue')
-			->with('core', 'shareapi_allow_links', 'yes')
-			->will($this->returnValue('yes'));
-
-		$this->request->expects($this->once())->method('getParam')->with('token')
-			->willReturn('token');
-		$this->shareManager->expects($this->once())->method('getShareByToken')
-			->with('token')->willReturn($share);
-
-		$share->expects($this->once())->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_LINK);
-
 		$controller = $this->createMock(ShareController::class);
 
 		$this->sharingCheckMiddleware->beforeController($controller, 'myMethod');
@@ -262,33 +200,6 @@ class SharingCheckMiddlewareTest extends \Test\TestCase {
 
 	/**
 	 * @expectedException \OCP\Files\NotFoundException
-	 * @expectedExceptionMessage Link sharing is disabled
-	 */
-	public function testBeforeControllerWithShareControllerWithSharingEnabledAPIDisabled() {
-
-		$share = $this->createMock(IShare::class);
-
-		$this->appManager
-			->expects($this->once())
-			->method('isEnabledForUser')
-			->with('files_sharing')
-			->will($this->returnValue(true));
-
-		$controller = $this->createMock(ShareController::class);
-
-		$this->request->expects($this->once())->method('getParam')->with('token')
-			->willReturn('token');
-		$this->shareManager->expects($this->once())->method('getShareByToken')
-			->with('token')->willReturn($share);
-
-		$share->expects($this->once())->method('getShareType')->willReturn(\OCP\Share::SHARE_TYPE_LINK);
-
-
-		$this->sharingCheckMiddleware->beforeController($controller, 'myMethod');
-	}
-
-	/**
-	 * @expectedException \OCP\Files\NotFoundException
 	 * @expectedExceptionMessage Sharing is disabled.
 	 */
 	public function testBeforeControllerWithSharingDisabled() {