sharingcheckmiddleware now handles externalshares as well

Added new annotations for the externalsharescontroller class
* @NoOutgoingFederatedSharingRequired
* @NoIncomingFederatedSharingRequired

By default both are required for all functions in the
externalSharesController.

A proper exception is thrown and then a 405 is returned instead of the
default error page. Since it is only an API endpoint this makes more
sense.

Unit tests added and updated

8 files changed, 279 insertions, 34 deletions

diff --git a/apps/files_sharing/appinfo/app.php b/apps/files_sharing/appinfo/app.php
index 03d5d72736e..a5c2f3c6f8d 100644
--- a/apps/files_sharing/appinfo/app.php
+++ b/apps/files_sharing/appinfo/app.php
@@ -39,9 +39,6 @@ $l = \OC::$server->getL10N('files_sharing');
 \OC::$CLASSPATH['OCA\Files\Share\Maintainer'] = 'files_sharing/lib/maintainer.php';
 \OC::$CLASSPATH['OCA\Files\Share\Proxy'] = 'files_sharing/lib/proxy.php';
 
-// Exceptions
-\OC::$CLASSPATH['OCA\Files_Sharing\Exceptions\BrokenPath'] = 'files_sharing/lib/exceptions.php';
-
 $application = new Application();
 $application->registerMountProviders();
 $application->setupPropagation();
diff --git a/apps/files_sharing/appinfo/application.php b/apps/files_sharing/appinfo/application.php
index b9c2844d78c..1d31bca2934 100644
--- a/apps/files_sharing/appinfo/application.php
+++ b/apps/files_sharing/appinfo/application.php
@@ -59,7 +59,6 @@ class Application extends App {
 			return new ExternalSharesController(
 				$c->query('AppName'),
 				$c->query('Request'),
-				$c->query('IsIncomingShareEnabled'),
 				$c->query('ExternalManager')
 			);
 		});
@@ -76,9 +75,6 @@ class Application extends App {
 		$container->registerService('UserManager', function (SimpleContainer $c) use ($server) {
 			return $server->getUserManager();
 		});
-		$container->registerService('IsIncomingShareEnabled', function (SimpleContainer $c) {
-			return Helper::isIncomingServer2serverShareEnabled();
-		});
 		$container->registerService('ExternalManager', function (SimpleContainer $c) use ($server) {
 			$user = $server->getUserSession()->getUser();
 			$uid = $user ? $user->getUID() : null;
@@ -98,7 +94,8 @@ class Application extends App {
 			return new SharingCheckMiddleware(
 				$c->query('AppName'),
 				$server->getConfig(),
-				$server->getAppManager()
+				$server->getAppManager(),
+				$c['ControllerMethodReflector']
 			);
 		});
 
diff --git a/apps/files_sharing/lib/controllers/externalsharescontroller.php b/apps/files_sharing/lib/controllers/externalsharescontroller.php
index 494a544b2b8..82b45eae6b8 100644
--- a/apps/files_sharing/lib/controllers/externalsharescontroller.php
+++ b/apps/files_sharing/lib/controllers/externalsharescontroller.php
@@ -36,8 +36,6 @@ use OCP\AppFramework\Http\JSONResponse;
  */
 class ExternalSharesController extends Controller {
 
-	/** @var bool */
-	private $incomingShareEnabled;
 	/** @var \OCA\Files_Sharing\External\Manager */
 	private $externalManager;
 
@@ -49,52 +47,42 @@ class ExternalSharesController extends Controller {
 	 */
 	public function __construct($appName,
 								IRequest $request,
-								$incomingShareEnabled,
 								\OCA\Files_Sharing\External\Manager $externalManager) {
 		parent::__construct($appName, $request);
-		$this->incomingShareEnabled = $incomingShareEnabled;
 		$this->externalManager = $externalManager;
 	}
 
 	/**
 	 * @NoAdminRequired
+	 * @NoOutgoingFederatedSharingRequired
 	 *
 	 * @return JSONResponse
 	 */
 	public function index() {
-		$shares = [];
-		if ($this->incomingShareEnabled) {
-			$shares = $this->externalManager->getOpenShares();
-		}
-		return new JSONResponse($shares);
+		return new JSONResponse($this->externalManager->getOpenShares());
 	}
 
 	/**
 	 * @NoAdminRequired
+	 * @NoOutgoingFederatedSharingRequired
 	 *
 	 * @param int $id
 	 * @return JSONResponse
 	 */
 	public function create($id) {
-		if ($this->incomingShareEnabled) {
-			$this->externalManager->acceptShare($id);
-		}
-
+		$this->externalManager->acceptShare($id);
 		return new JSONResponse();
 	}
 
 	/**
 	 * @NoAdminRequired
+	 * @NoOutgoingFederatedSharingRequired
 	 *
 	 * @param $id
 	 * @return JSONResponse
 	 */
 	public function destroy($id) {
-		if ($this->incomingShareEnabled) {
-			$this->externalManager->declineShare($id);
-		}
-
+		$this->externalManager->declineShare($id);
 		return new JSONResponse();
 	}
-
 }
diff --git a/apps/files_sharing/lib/exceptions.php b/apps/files_sharing/lib/exceptions/brokenpath.php
index 71fe93f8e92..71fe93f8e92 100644
--- a/apps/files_sharing/lib/exceptions.php
+++ b/apps/files_sharing/lib/exceptions/brokenpath.php
diff --git a/apps/files_sharing/lib/exceptions/s2sexception.php b/apps/files_sharing/lib/exceptions/s2sexception.php
new file mode 100644
index 00000000000..7f5557f8cd3
--- /dev/null
+++ b/apps/files_sharing/lib/exceptions/s2sexception.php
@@ -0,0 +1,26 @@
+<?php
+/**
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OCA\Files_Sharing\Exceptions;
+
+/**
+ * S2S sharing not allowed
+ */
+class S2SException extends \Exception {
+}
diff --git a/apps/files_sharing/lib/middleware/sharingcheckmiddleware.php b/apps/files_sharing/lib/middleware/sharingcheckmiddleware.php
index f51399b76a8..942efc0483e 100644
--- a/apps/files_sharing/lib/middleware/sharingcheckmiddleware.php
+++ b/apps/files_sharing/lib/middleware/sharingcheckmiddleware.php
@@ -29,6 +29,9 @@ use OCP\AppFramework\Middleware;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\Files\NotFoundException;
 use OCP\IConfig;
+use OCP\AppFramework\Utility\IControllerMethodReflector;
+use OCA\Files_Sharing\Exceptions\S2SException;
+use OCP\AppFramework\Http\JSONResponse;
 
 /**
  * Checks whether the "sharing check" is enabled
@@ -43,6 +46,8 @@ class SharingCheckMiddleware extends Middleware {
 	protected $config;
 	/** @var IAppManager */
 	protected $appManager;
+	/** @var IControllerMethodReflector */
+	protected $reflector;
 
 	/***
 	 * @param string $appName
@@ -51,10 +56,13 @@ class SharingCheckMiddleware extends Middleware {
 	 */
 	public function __construct($appName,
 								IConfig $config,
-								IAppManager $appManager) {
+								IAppManager $appManager,
+								IControllerMethodReflector $reflector
+								) {
 		$this->appName = $appName;
 		$this->config = $config;
 		$this->appManager = $appManager;
+		$this->reflector = $reflector;
 	}
 
 	/**
@@ -69,8 +77,9 @@ class SharingCheckMiddleware extends Middleware {
 			throw new NotFoundException('Sharing is disabled.');
 		}
 
-		if ($controller instanceof \OCA\Files_Sharing\Controllers\ExternalSharesController) {
-			//TODO: Proper checks
+		if ($controller instanceof \OCA\Files_Sharing\Controllers\ExternalSharesController &&
+			!$this->externalSharesChecks()) {
+			throw new S2SException('Federated sharing not allowed');
 		} else if ($controller instanceof \OCA\Files_Sharing\Controllers\ShareController &&
 			!$this->isLinkSharingEnabled()) {
 			throw new NotFoundException('Link sharing is disabled');
@@ -91,10 +100,33 @@ class SharingCheckMiddleware extends Middleware {
 			return new NotFoundResponse();
 		}
 
+		if (is_a($exception, '\OCA\Files_Sharing\Exceptions\S2SException')) {
+			return new JSONResponse($exception->getMessage(), 405);
+		}
+
 		throw $exception;
 	}
 
 	/**
+	 * Checks for externalshares controller
+	 * @return bool
+	 */
+	private function externalSharesChecks() {
+
+		if (!$this->reflector->hasAnnotation('NoIncomingFederatedSharingRequired') &&
+			$this->config->getAppValue('files_sharing', 'incoming_server2server_share_enabled', 'yes') !== 'yes') {
+			return false;
+		}
+
+		if (!$this->reflector->hasAnnotation('NoOutgoingFederatedSharingRequired') &&
+		    $this->config->getAppValue('files_sharing', 'outgoing_server2server_share_enabled', 'yes') !== 'yes') {
+			return false;
+		}
+
+		return true;
+	}
+
+	/**
 	 * Check whether sharing is enabled
 	 * @return bool
 	 */
diff --git a/apps/files_sharing/tests/controller/externalsharecontroller.php b/apps/files_sharing/tests/controller/externalsharecontroller.php
new file mode 100644
index 00000000000..3a082a8136a
--- /dev/null
+++ b/apps/files_sharing/tests/controller/externalsharecontroller.php
@@ -0,0 +1,89 @@
+<?php
+/**
+ * @author Lukas Reschke <lukas@owncloud.com>
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OCA\Files_Sharing\Controllers;
+
+use OCP\AppFramework\Http\DataResponse;
+use OCP\AppFramework\Http\JSONResponse;
+use OCP\Http\Client\IClientService;
+use OCP\IRequest;
+
+/**
+ * Class ExternalShareControllerTest
+ *
+ * @package OCA\Files_Sharing\Controllers
+ */
+class ExternalShareControllerTest extends \Test\TestCase {
+	/** @var IRequest */
+	private $request;
+	/** @var \OCA\Files_Sharing\External\Manager */
+	private $externalManager;
+	/** @var IClientService */
+	private $clientService;
+
+	public function setUp() {
+		$this->request = $this->getMockBuilder('\\OCP\\IRequest')
+			->disableOriginalConstructor()->getMock();
+		$this->externalManager = $this->getMockBuilder('\\OCA\\Files_Sharing\\External\\Manager')
+			->disableOriginalConstructor()->getMock();
+		$this->clientService = $this->getMockBuilder('\\OCP\Http\\Client\\IClientService')
+			->disableOriginalConstructor()->getMock();
+	}
+
+	/**
+	 * @return ExternalSharesController
+	 */
+	public function getExternalShareController() {
+		return new ExternalSharesController(
+			'files_sharing',
+			$this->request,
+			$this->externalManager,
+			$this->clientService
+		);
+	}
+
+	public function testIndex() {
+		$this->externalManager
+			->expects($this->once())
+			->method('getOpenShares')
+			->will($this->returnValue(['MyDummyArray']));
+
+		$this->assertEquals(new JSONResponse(['MyDummyArray']), $this->getExternalShareController()->index());
+	}
+
+	public function testCreate() {
+		$this->externalManager
+			->expects($this->once())
+			->method('acceptShare')
+			->with(4);
+
+		$this->assertEquals(new JSONResponse(), $this->getExternalShareController()->create(4));
+	}
+
+	public function testDestroy() {
+		$this->externalManager
+			->expects($this->once())
+			->method('declineShare')
+			->with(4);
+
+		$this->assertEquals(new JSONResponse(), $this->getExternalShareController()->destroy(4));
+	}
+}
diff --git a/apps/files_sharing/tests/middleware/sharingcheckmiddleware.php b/apps/files_sharing/tests/middleware/sharingcheckmiddleware.php
index a43b11c81b6..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,8 +52,14 @@ 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 testIsSharingEnabledWithAppEnabled() {
@@ -114,7 +125,93 @@ class SharingCheckMiddlewareTest extends \Test\TestCase {
 		$this->assertFalse(self::invokePrivate($this->sharingCheckMiddleware, 'isLinkSharingEnabled'));
 	}
 
-	public function testBeforeControllerWithSharingEnabled() {
+	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
+			->method('getAppValue')
+			->will($this->returnValueMap($config));
+
+		$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 testBeforeControllerWithShareControllerWithSharingEnabled() {
 		$this->appManager
 			->expects($this->once())
 			->method('isEnabledForUser')
@@ -141,14 +238,14 @@ class SharingCheckMiddlewareTest extends \Test\TestCase {
 
 	/**
 	 * @expectedException \OCP\Files\NotFoundException
-	 * @expectedExceptionMessage Sharing is disabled.
+	 * @expectedExceptionMessage Link sharing is disabled
 	 */
-	public function testBeforeControllerWithSharingDisabled() {
+	public function testBeforeControllerWithShareControllerWithSharingEnabledAPIDisabled() {
 		$this->appManager
 			->expects($this->once())
 			->method('isEnabledForUser')
 			->with('files_sharing')
-			->will($this->returnValue(false));
+			->will($this->returnValue(true));
 
 		$controller = $this->getMockBuilder('\OCA\Files_Sharing\Controllers\ShareController')
 			->disableOriginalConstructor()->getMock();
@@ -157,6 +254,20 @@ class SharingCheckMiddlewareTest extends \Test\TestCase {
 	}
 
 	/**
+	 * @expectedException \OCP\Files\NotFoundException
+	 * @expectedExceptionMessage Sharing is disabled.
+	 */
+	public function testBeforeControllerWithSharingDisabled() {
+		$this->appManager
+			->expects($this->once())
+			->method('isEnabledForUser')
+			->with('files_sharing')
+			->will($this->returnValue(false));
+
+		$this->sharingCheckMiddleware->beforeController($this->controllerMock, 'myMethod');
+	}
+
+	/**
 	 * @expectedException \Exception
 	 * @expectedExceptionMessage My Exception message
 	 */
@@ -168,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')));
+	}
+
+
 }