Merge pull request #19487 from owncloud/split_share_middleware

Split files_sharing middelware
This commit is contained in:
Thomas Müller 2015-10-05 10:57:11 +02:00
commit 710b7dd81c
8 changed files with 233 additions and 90 deletions

View File

@ -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();

View File

@ -61,7 +61,6 @@ class Application extends App {
return new ExternalSharesController(
$c->query('AppName'),
$c->query('Request'),
$c->query('IsIncomingShareEnabled'),
$c->query('ExternalManager'),
$c->query('HttpClientService')
);
@ -82,9 +81,6 @@ class Application extends App {
$container->registerService('HttpClientService', function (SimpleContainer $c) use ($server) {
return $server->getHTTPClientService();
});
$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;
@ -105,7 +101,8 @@ class Application extends App {
return new SharingCheckMiddleware(
$c->query('AppName'),
$server->getConfig(),
$server->getAppManager()
$server->getAppManager(),
$c['ControllerMethodReflector']
);
});

View File

@ -36,8 +36,6 @@ use OCP\AppFramework\Http\DataResponse;
*/
class ExternalSharesController extends Controller {
/** @var bool */
private $incomingShareEnabled;
/** @var \OCA\Files_Sharing\External\Manager */
private $externalManager;
/** @var IClientService */
@ -52,53 +50,44 @@ class ExternalSharesController extends Controller {
*/
public function __construct($appName,
IRequest $request,
$incomingShareEnabled,
\OCA\Files_Sharing\External\Manager $externalManager,
IClientService $clientService) {
parent::__construct($appName, $request);
$this->incomingShareEnabled = $incomingShareEnabled;
$this->externalManager = $externalManager;
$this->clientService = $clientService;
}
/**
* @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();
}
@ -127,6 +116,8 @@ class ExternalSharesController extends Controller {
/**
* @PublicPage
* @NoOutgoingFederatedSharingRequired
* @NoIncomingFederatedSharingRequired
*
* @param string $remote
* @return DataResponse

View File

@ -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 {
}

View File

@ -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;
}
/**
@ -68,6 +76,14 @@ class SharingCheckMiddleware extends Middleware {
if(!$this->isSharingEnabled()) {
throw new NotFoundException('Sharing is disabled.');
}
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');
}
}
/**
@ -84,9 +100,32 @@ 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
@ -98,6 +137,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;

View File

@ -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')

View File

@ -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,29 +52,23 @@ 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->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 +82,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,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() {
$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() {
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')
@ -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')));
}
}