Merge pull request #19487 from owncloud/split_share_middleware

Split files_sharing middelware

2 files changed, 145 insertions, 60 deletions

diff --git a/apps/files_sharing/tests/controller/externalsharecontroller.php b/apps/files_sharing/tests/controller/externalsharecontroller.php
index 3dc34bca7a4..7b3e1af4c74 100644
--- a/apps/files_sharing/tests/controller/externalsharecontroller.php
+++ b/apps/files_sharing/tests/controller/externalsharecontroller.php
@@ -32,8 +32,6 @@ use OCP\IRequest;
  * @package OCA\Files_Sharing\Controllers
  */
 class ExternalShareControllerTest extends \Test\TestCase {
-	/** @var bool */
-	private $incomingShareEnabled;
 	/** @var IRequest */
 	private $request;
 	/** @var \OCA\Files_Sharing\External\Manager */
@@ -57,22 +55,12 @@ class ExternalShareControllerTest extends \Test\TestCase {
 		return new ExternalSharesController(
 			'files_sharing',
 			$this->request,
-			$this->incomingShareEnabled,
 			$this->externalManager,
 			$this->clientService
 		);
 	}
 
-	public function testIndexDisabled() {
-		$this->externalManager
-			->expects($this->never())
-			->method('getOpenShares');
-
-		$this->assertEquals(new JSONResponse(), $this->getExternalShareController()->index());
-	}
-
-	public function testIndexEnabled() {
-		$this->incomingShareEnabled = true;
+	public function testIndex() {
 		$this->externalManager
 			->expects($this->once())
 			->method('getOpenShares')
@@ -81,16 +69,7 @@ class ExternalShareControllerTest extends \Test\TestCase {
 		$this->assertEquals(new JSONResponse(['MyDummyArray']), $this->getExternalShareController()->index());
 	}
 
-	public function testCreateDisabled() {
-		$this->externalManager
-			->expects($this->never())
-			->method('acceptShare');
-
-		$this->assertEquals(new JSONResponse(), $this->getExternalShareController()->create(4));
-	}
-
-	public function testCreateEnabled() {
-		$this->incomingShareEnabled = true;
+	public function testCreate() {
 		$this->externalManager
 			->expects($this->once())
 			->method('acceptShare')
@@ -99,16 +78,7 @@ class ExternalShareControllerTest extends \Test\TestCase {
 		$this->assertEquals(new JSONResponse(), $this->getExternalShareController()->create(4));
 	}
 
-	public function testDestroyDisabled() {
-		$this->externalManager
-			->expects($this->never())
-			->method('destroy');
-
-		$this->assertEquals(new JSONResponse(), $this->getExternalShareController()->destroy(4));
-	}
-
-	public function testDestroyEnabled() {
-		$this->incomingShareEnabled = true;
+	public function testDestroy() {
 		$this->externalManager
 			->expects($this->once())
 			->method('declineShare')
diff --git a/apps/files_sharing/tests/middleware/sharingcheckmiddleware.php b/apps/files_sharing/tests/middleware/sharingcheckmiddleware.php
index 3171d45d331..05cb7497680 100644
--- a/apps/files_sharing/tests/middleware/sharingcheckmiddleware.php
+++ b/apps/files_sharing/tests/middleware/sharingcheckmiddleware.php
@@ -25,6 +25,9 @@
 namespace OCA\Files_Sharing\Middleware;
 use OCP\AppFramework\Http\NotFoundResponse;
 use OCP\Files\NotFoundException;
+use OCP\AppFramework\Utility\IControllerMethodReflector;
+use OCA\Files_Sharing\Exceptions\S2SException;
+use OCP\AppFramework\Http\JSONResponse;
 
 /**
  * @package OCA\Files_Sharing\Middleware\SharingCheckMiddleware
@@ -39,6 +42,8 @@ class SharingCheckMiddlewareTest extends \Test\TestCase {
 	private $sharingCheckMiddleware;
 	/** @var \OCP\AppFramework\Controller */
 	private $controllerMock;
+	/** @var IControllerMethodReflector */
+	private $reflector;
 
 	protected function setUp() {
 		$this->config = $this->getMockBuilder('\OCP\IConfig')
@@ -47,17 +52,37 @@ class SharingCheckMiddlewareTest extends \Test\TestCase {
 			->disableOriginalConstructor()->getMock();
 		$this->controllerMock = $this->getMockBuilder('\OCP\AppFramework\Controller')
 			->disableOriginalConstructor()->getMock();
+		$this->reflector = $this->getMockBuilder('\OCP\AppFramework\Utility\IControllerMethodReflector')
+			->disableOriginalConstructor()->getMock();
 
-		$this->sharingCheckMiddleware = new SharingCheckMiddleware('files_sharing', $this->config, $this->appManager);
+		$this->sharingCheckMiddleware = new SharingCheckMiddleware(
+			'files_sharing',
+			$this->config,
+			$this->appManager,
+			$this->reflector);
 	}
 
-	public function testIsSharingEnabledWithEverythingEnabled() {
+	public function testIsSharingEnabledWithAppEnabled() {
 		$this->appManager
 			->expects($this->once())
 			->method('isEnabledForUser')
 			->with('files_sharing')
 			->will($this->returnValue(true));
 
+		$this->assertTrue(self::invokePrivate($this->sharingCheckMiddleware, 'isSharingEnabled'));
+	}
+
+	public function testIsSharingEnabledWithAppDisabled() {
+		$this->appManager
+			->expects($this->once())
+			->method('isEnabledForUser')
+			->with('files_sharing')
+			->will($this->returnValue(false));
+
+		$this->assertFalse(self::invokePrivate($this->sharingCheckMiddleware, 'isSharingEnabled'));
+	}
+
+	public function testIsLinkSharingEnabledWithEverythinEnabled() {
 		$this->config
 			->expects($this->at(0))
 			->method('getAppValue')
@@ -70,26 +95,11 @@ class SharingCheckMiddlewareTest extends \Test\TestCase {
 			->with('core', 'shareapi_allow_links', 'yes')
 			->will($this->returnValue('yes'));
 
-		$this->assertTrue(self::invokePrivate($this->sharingCheckMiddleware, 'isSharingEnabled'));
+		$this->assertTrue(self::invokePrivate($this->sharingCheckMiddleware, 'isLinkSharingEnabled'));
 	}
 
-	public function testIsSharingEnabledWithAppDisabled() {
-		$this->appManager
-			->expects($this->once())
-			->method('isEnabledForUser')
-			->with('files_sharing')
-			->will($this->returnValue(false));
-
-		$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 testIsLinkSharingEnabledWithLinkSharingDisabled() {
 		$this->config
 			->expects($this->at(0))
 			->method('getAppValue')
@@ -102,26 +112,106 @@ 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() {
+	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 = [];
+
+		foreach ([false, true] as $annIn) {
+			foreach ([false, true] as $annOut) {
+				foreach ([false, true] as $confIn) {
+					foreach ([false, true] as $confOut) {
+
+						$res = true;
+						if (!$annIn && !$confIn) {
+							$res = false;
+						} elseif (!$annOut && !$confOut) {
+							$res = false;
+						}
+
+						$d = [
+							[
+								['NoIncomingFederatedSharingRequired', $annIn],
+								['NoOutgoingFederatedSharingRequired', $annOut],
+							],
+							[
+								['files_sharing', 'incoming_server2server_share_enabled', 'yes', $confIn ? 'yes' : 'no'],
+								['files_sharing', 'outgoing_server2server_share_enabled', 'yes', $confOut ? 'yes' : 'no'],
+							],
+							$res
+						];
+
+						$data[] = $d;
+					}
+				}
+			}
+		}
+
+		return $data;
+	}
+
+	/**
+	 * @dataProvider externalSharesChecksDataProvider
+	 */
+	public function testExternalSharesChecks($annotations, $config, $expectedResult) {
+		$this->reflector
+			->expects($this->atLeastOnce())
+			->method('hasAnnotation')
+			->will($this->returnValueMap($annotations));
+
+		$this->config
+			->method('getAppValue')
+			->will($this->returnValueMap($config));
+
+		$this->assertEquals($expectedResult, self::invokePrivate($this->sharingCheckMiddleware, 'externalSharesChecks'));
+	}
+
+	/**
+	 * @dataProvider externalSharesChecksDataProvider
+	 */
+	public function testBeforeControllerWithExternalShareControllerWithSharingEnabled($annotations, $config, $noException) {
 		$this->appManager
 			->expects($this->once())
 			->method('isEnabledForUser')
 			->with('files_sharing')
 			->will($this->returnValue(true));
 
+		$this->reflector
+			->expects($this->atLeastOnce())
+			->method('hasAnnotation')
+			->will($this->returnValueMap($annotations));
+
 		$this->config
-			->expects($this->once())
 			->method('getAppValue')
-			->with('core', 'shareapi_enabled', 'yes')
-			->will($this->returnValue('no'));
+			->will($this->returnValueMap($config));
 
-		$this->assertFalse(self::invokePrivate($this->sharingCheckMiddleware, 'isSharingEnabled'));
+		$controller = $this->getMockBuilder('\OCA\Files_Sharing\Controllers\ExternalSharesController')
+			->disableOriginalConstructor()->getMock();
+
+		$exceptionThrown = false;
+
+		try {
+			$this->sharingCheckMiddleware->beforeController($controller, 'myMethod');
+		} catch (\OCA\Files_Sharing\Exceptions\S2SException $exception) {
+			$exceptionThrown = true;
+		}
+
+		$this->assertNotEquals($noException, $exceptionThrown);
 	}
 
-	public function testBeforeControllerWithSharingEnabled() {
+	public function testBeforeControllerWithShareControllerWithSharingEnabled() {
 		$this->appManager
 			->expects($this->once())
 			->method('isEnabledForUser')
@@ -140,7 +230,27 @@ 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');
+	}
+
+	/**
+	 * @expectedException \OCP\Files\NotFoundException
+	 * @expectedExceptionMessage Link sharing is disabled
+	 */
+	public function testBeforeControllerWithShareControllerWithSharingEnabledAPIDisabled() {
+		$this->appManager
+			->expects($this->once())
+			->method('isEnabledForUser')
+			->with('files_sharing')
+			->will($this->returnValue(true));
+
+		$controller = $this->getMockBuilder('\OCA\Files_Sharing\Controllers\ShareController')
+			->disableOriginalConstructor()->getMock();
+
+		$this->sharingCheckMiddleware->beforeController($controller, 'myMethod');
 	}
 
 	/**
@@ -169,4 +279,9 @@ class SharingCheckMiddlewareTest extends \Test\TestCase {
 		$this->assertEquals(new NotFoundResponse(), $this->sharingCheckMiddleware->afterException($this->controllerMock, 'myMethod', new NotFoundException('My Exception message')));
 	}
 
+	public function testAfterExceptionWithS2SException() {
+		$this->assertEquals(new JSONResponse('My Exception message', 405), $this->sharingCheckMiddleware->afterException($this->controllerMock, 'myMethod', new S2SException('My Exception message')));
+	}
+
+
 }