From dc38e674a5d547e7fd53d66fb0ac0dbb5490ea77 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 30 Sep 2015 21:35:52 +0200 Subject: [PATCH] Split files_sharing middelware Since for external shares there is no need for link shares to be enabled we should check which controller is actually being called. This makes sure that in all cases we verify that the files_sharing app is enabled. But only for the share controller (public shares) we check if the API is enabled and if links are enabled. TODO: add checks for federated sharing as well --- .../lib/middleware/sharingcheckmiddleware.php | 15 +++++ .../middleware/sharingcheckmiddleware.php | 59 +++++++++---------- 2 files changed, 44 insertions(+), 30 deletions(-) diff --git a/apps/files_sharing/lib/middleware/sharingcheckmiddleware.php b/apps/files_sharing/lib/middleware/sharingcheckmiddleware.php index 61dfd914d0..f51399b76a 100644 --- a/apps/files_sharing/lib/middleware/sharingcheckmiddleware.php +++ b/apps/files_sharing/lib/middleware/sharingcheckmiddleware.php @@ -68,6 +68,13 @@ class SharingCheckMiddleware extends Middleware { if(!$this->isSharingEnabled()) { throw new NotFoundException('Sharing is disabled.'); } + + if ($controller instanceof \OCA\Files_Sharing\Controllers\ExternalSharesController) { + //TODO: Proper checks + } else if ($controller instanceof \OCA\Files_Sharing\Controllers\ShareController && + !$this->isLinkSharingEnabled()) { + throw new NotFoundException('Link sharing is disabled'); + } } /** @@ -98,6 +105,14 @@ class SharingCheckMiddleware extends Middleware { return false; } + return true; + } + + /** + * Check if link sharing is allowed + * @return bool + */ + private function isLinkSharingEnabled() { // Check if the shareAPI is enabled if ($this->config->getAppValue('core', 'shareapi_enabled', 'yes') !== 'yes') { return false; diff --git a/apps/files_sharing/tests/middleware/sharingcheckmiddleware.php b/apps/files_sharing/tests/middleware/sharingcheckmiddleware.php index 3171d45d33..a43b11c81b 100644 --- a/apps/files_sharing/tests/middleware/sharingcheckmiddleware.php +++ b/apps/files_sharing/tests/middleware/sharingcheckmiddleware.php @@ -51,25 +51,13 @@ class SharingCheckMiddlewareTest extends \Test\TestCase { $this->sharingCheckMiddleware = new SharingCheckMiddleware('files_sharing', $this->config, $this->appManager); } - public function testIsSharingEnabledWithEverythingEnabled() { + public function testIsSharingEnabledWithAppEnabled() { $this->appManager ->expects($this->once()) ->method('isEnabledForUser') ->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->assertTrue(self::invokePrivate($this->sharingCheckMiddleware, 'isSharingEnabled')); } @@ -83,13 +71,24 @@ class SharingCheckMiddlewareTest extends \Test\TestCase { $this->assertFalse(self::invokePrivate($this->sharingCheckMiddleware, 'isSharingEnabled')); } - public function testIsSharingEnabledWithLinkSharingDisabled() { - $this->appManager - ->expects($this->once()) - ->method('isEnabledForUser') - ->with('files_sharing') - ->will($this->returnValue(true)); + 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') @@ -102,23 +101,17 @@ class SharingCheckMiddlewareTest extends \Test\TestCase { ->with('core', 'shareapi_allow_links', 'yes') ->will($this->returnValue('no')); - $this->assertFalse(self::invokePrivate($this->sharingCheckMiddleware, 'isSharingEnabled')); + $this->assertFalse(self::invokePrivate($this->sharingCheckMiddleware, 'isLinkSharingEnabled')); } - public function testIsSharingEnabledWithSharingAPIDisabled() { - $this->appManager - ->expects($this->once()) - ->method('isEnabledForUser') - ->with('files_sharing') - ->will($this->returnValue(true)); - + 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, 'isSharingEnabled')); + $this->assertFalse(self::invokePrivate($this->sharingCheckMiddleware, 'isLinkSharingEnabled')); } public function testBeforeControllerWithSharingEnabled() { @@ -140,7 +133,10 @@ class SharingCheckMiddlewareTest extends \Test\TestCase { ->with('core', 'shareapi_allow_links', 'yes') ->will($this->returnValue('yes')); - $this->sharingCheckMiddleware->beforeController($this->controllerMock, 'myMethod'); + $controller = $this->getMockBuilder('\OCA\Files_Sharing\Controllers\ShareController') + ->disableOriginalConstructor()->getMock(); + + $this->sharingCheckMiddleware->beforeController($controller, 'myMethod'); } /** @@ -154,7 +150,10 @@ class SharingCheckMiddlewareTest extends \Test\TestCase { ->with('files_sharing') ->will($this->returnValue(false)); - $this->sharingCheckMiddleware->beforeController($this->controllerMock, 'myMethod'); + $controller = $this->getMockBuilder('\OCA\Files_Sharing\Controllers\ShareController') + ->disableOriginalConstructor()->getMock(); + + $this->sharingCheckMiddleware->beforeController($controller, 'myMethod'); } /**