diff --git a/apps/files_sharing/lib/AppInfo/Application.php b/apps/files_sharing/lib/AppInfo/Application.php index 922db7dac7..403d30ae2e 100644 --- a/apps/files_sharing/lib/AppInfo/Application.php +++ b/apps/files_sharing/lib/AppInfo/Application.php @@ -111,7 +111,9 @@ class Application extends App { $c->query('AppName'), $server->getConfig(), $server->getAppManager(), - $c['ControllerMethodReflector'] + $c['ControllerMethodReflector'], + $server->getShareManager(), + $server->getRequest() ); }); diff --git a/apps/files_sharing/lib/Middleware/SharingCheckMiddleware.php b/apps/files_sharing/lib/Middleware/SharingCheckMiddleware.php index 7e9109bf2d..5712b96b97 100644 --- a/apps/files_sharing/lib/Middleware/SharingCheckMiddleware.php +++ b/apps/files_sharing/lib/Middleware/SharingCheckMiddleware.php @@ -25,6 +25,8 @@ namespace OCA\Files_Sharing\Middleware; +use OCA\Files_Sharing\Controller\ExternalSharesController; +use OCA\Files_Sharing\Controller\ShareController; use OCP\App\IAppManager; use OCP\AppFramework\Http\NotFoundResponse; use OCP\AppFramework\Middleware; @@ -33,6 +35,8 @@ use OCP\IConfig; use OCP\AppFramework\Utility\IControllerMethodReflector; use OCA\Files_Sharing\Exceptions\S2SException; use OCP\AppFramework\Http\JSONResponse; +use OCP\IRequest; +use OCP\Share\IManager; /** * Checks whether the "sharing check" is enabled @@ -49,21 +53,32 @@ class SharingCheckMiddleware extends Middleware { protected $appManager; /** @var IControllerMethodReflector */ protected $reflector; + /** @var IManager */ + protected $shareManager; + /** @var IRequest */ + protected $request; /*** * @param string $appName * @param IConfig $config * @param IAppManager $appManager + * @param IControllerMethodReflector $reflector + * @param IManager $shareManager + * @param IRequest $request */ public function __construct($appName, IConfig $config, IAppManager $appManager, - IControllerMethodReflector $reflector + IControllerMethodReflector $reflector, + IManager $shareManager, + IRequest $request ) { $this->appName = $appName; $this->config = $config; $this->appManager = $appManager; $this->reflector = $reflector; + $this->shareManager = $shareManager; + $this->request = $request; } /** @@ -72,18 +87,23 @@ class SharingCheckMiddleware extends Middleware { * @param \OCP\AppFramework\Controller $controller * @param string $methodName * @throws NotFoundException + * @throws S2SException */ public function beforeController($controller, $methodName) { if(!$this->isSharingEnabled()) { throw new NotFoundException('Sharing is disabled.'); } - if ($controller instanceof \OCA\Files_Sharing\Controller\ExternalSharesController && + if ($controller instanceof ExternalSharesController && !$this->externalSharesChecks()) { throw new S2SException('Federated sharing not allowed'); - } else if ($controller instanceof \OCA\Files_Sharing\Controller\ShareController && - !$this->isLinkSharingEnabled()) { - throw new NotFoundException('Link sharing is disabled'); + } else if ($controller instanceof ShareController) { + $token = $this->request->getParam('token'); + $share = $this->shareManager->getShareByToken($token); + if ($share->getShareType() === \OCP\Share::SHARE_TYPE_LINK + && !$this->isLinkSharingEnabled()) { + throw new NotFoundException('Link sharing is disabled'); + } } } diff --git a/apps/files_sharing/tests/Middleware/SharingCheckMiddlewareTest.php b/apps/files_sharing/tests/Middleware/SharingCheckMiddlewareTest.php index c2965d04b6..8d7d42722b 100644 --- a/apps/files_sharing/tests/Middleware/SharingCheckMiddlewareTest.php +++ b/apps/files_sharing/tests/Middleware/SharingCheckMiddlewareTest.php @@ -34,6 +34,9 @@ use OCP\AppFramework\Utility\IControllerMethodReflector; use OCA\Files_Sharing\Exceptions\S2SException; use OCP\AppFramework\Http\JSONResponse; use OCP\IConfig; +use OCP\IRequest; +use OCP\Share\IManager; +use OCP\Share\IShare; /** * @package OCA\Files_Sharing\Middleware\SharingCheckMiddleware @@ -50,6 +53,10 @@ class SharingCheckMiddlewareTest extends \Test\TestCase { private $controllerMock; /** @var IControllerMethodReflector|\PHPUnit_Framework_MockObject_MockObject */ private $reflector; + /** @var IManager | \PHPUnit_Framework_MockObject_MockObject */ + private $shareManager; + /** @var IRequest | \PHPUnit_Framework_MockObject_MockObject */ + private $request; protected function setUp() { parent::setUp(); @@ -58,12 +65,16 @@ class SharingCheckMiddlewareTest extends \Test\TestCase { $this->appManager = $this->createMock(IAppManager::class); $this->controllerMock = $this->createMock(Controller::class); $this->reflector = $this->createMock(IControllerMethodReflector::class); + $this->shareManager = $this->createMock(IManager::class); + $this->request = $this->createMock(IRequest::class); $this->sharingCheckMiddleware = new SharingCheckMiddleware( 'files_sharing', $this->config, $this->appManager, - $this->reflector); + $this->reflector, + $this->shareManager, + $this->request); } public function testIsSharingEnabledWithAppEnabled() { @@ -215,6 +226,9 @@ class SharingCheckMiddlewareTest extends \Test\TestCase { } public function testBeforeControllerWithShareControllerWithSharingEnabled() { + + $share = $this->createMock(IShare::class); + $this->appManager ->expects($this->once()) ->method('isEnabledForUser') @@ -233,6 +247,13 @@ class SharingCheckMiddlewareTest extends \Test\TestCase { ->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'); @@ -243,6 +264,9 @@ class SharingCheckMiddlewareTest extends \Test\TestCase { * @expectedExceptionMessage Link sharing is disabled */ public function testBeforeControllerWithShareControllerWithSharingEnabledAPIDisabled() { + + $share = $this->createMock(IShare::class); + $this->appManager ->expects($this->once()) ->method('isEnabledForUser') @@ -251,6 +275,14 @@ class SharingCheckMiddlewareTest extends \Test\TestCase { $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'); } diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 6eab5e05a2..acc142f62b 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -587,7 +587,6 @@ class Manager implements IManager { $share->setPassword($this->hasher->hash($share->getPassword())); } } else if ($share->getShareType() === \OCP\Share::SHARE_TYPE_EMAIL) { - $this->linkCreateChecks($share); $share->setToken( $this->secureRandom->generate( \OC\Share\Constants::TOKEN_LENGTH, diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index 00009a73b0..7b01a8f9e7 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -1768,6 +1768,116 @@ class ManagerTest extends \Test\TestCase { $this->assertEquals('hashed', $share->getPassword()); } + public function testCreateShareMail() { + $manager = $this->createManagerMock() + ->setMethods([ + 'canShare', + 'generalCreateChecks', + 'linkCreateChecks', + 'pathCreateChecks', + 'validateExpirationDate', + 'verifyPassword', + 'setLinkParent', + ]) + ->getMock(); + + $shareOwner = $this->createMock(IUser::class); + $shareOwner->method('getUID')->willReturn('shareOwner'); + + $storage = $this->createMock(Storage::class); + $path = $this->createMock(File::class); + $path->method('getOwner')->willReturn($shareOwner); + $path->method('getName')->willReturn('target'); + $path->method('getId')->willReturn(1); + $path->method('getStorage')->willReturn($storage); + + $share = $this->manager->newShare(); + $share->setShareType(\OCP\Share::SHARE_TYPE_EMAIL) + ->setNode($path) + ->setSharedBy('sharedBy') + ->setPermissions(\OCP\Constants::PERMISSION_ALL); + + $manager->expects($this->once()) + ->method('canShare') + ->with($share) + ->willReturn(true); + $manager->expects($this->once()) + ->method('generalCreateChecks') + ->with($share);; + $manager->expects($this->never()) + ->method('linkCreateChecks'); + $manager->expects($this->once()) + ->method('pathCreateChecks') + ->with($path); + $manager->expects($this->never()) + ->method('validateExpirationDate'); + $manager->expects($this->never()) + ->method('verifyPassword'); + $manager->expects($this->never()) + ->method('setLinkParent'); + + $this->secureRandom->method('getMediumStrengthGenerator') + ->will($this->returnSelf()); + $this->secureRandom->method('generate') + ->willReturn('token'); + + $this->defaultProvider + ->expects($this->once()) + ->method('create') + ->with($share) + ->will($this->returnCallback(function(Share $share) { + return $share->setId(42); + })); + + $hookListner = $this->getMockBuilder('Dummy')->setMethods(['pre', 'post'])->getMock(); + \OCP\Util::connectHook('OCP\Share', 'pre_shared', $hookListner, 'pre'); + \OCP\Util::connectHook('OCP\Share', 'post_shared', $hookListner, 'post'); + + $hookListnerExpectsPre = [ + 'itemType' => 'file', + 'itemSource' => 1, + 'shareType' => \OCP\Share::SHARE_TYPE_EMAIL, + 'uidOwner' => 'sharedBy', + 'permissions' => 31, + 'fileSource' => 1, + 'expiration' => null, + 'token' => 'token', + 'run' => true, + 'error' => '', + 'itemTarget' => '/target', + 'shareWith' => null, + ]; + + $hookListnerExpectsPost = [ + 'itemType' => 'file', + 'itemSource' => 1, + 'shareType' => \OCP\Share::SHARE_TYPE_EMAIL, + 'uidOwner' => 'sharedBy', + 'permissions' => 31, + 'fileSource' => 1, + 'expiration' => null, + 'token' => 'token', + 'id' => 42, + 'itemTarget' => '/target', + 'fileTarget' => '/target', + 'shareWith' => null, + ]; + + $hookListner->expects($this->once()) + ->method('pre') + ->with($this->equalTo($hookListnerExpectsPre)); + $hookListner->expects($this->once()) + ->method('post') + ->with($this->equalTo($hookListnerExpectsPost)); + + /** @var IShare $share */ + $share = $manager->createShare($share); + + $this->assertSame('shareOwner', $share->getShareOwner()); + $this->assertEquals('/target', $share->getTarget()); + $this->assertEquals('token', $share->getToken()); + } + /** * @expectedException Exception * @expectedExceptionMessage I won't let you share